[Home]

Summary:ASTERISK-04047: [patch] astobj.h documentation
Reporter:Matthew Nicholson (mnicholson)Labels:
Date Opened:2005-05-03 14:14:30Date Closed:2008-01-15 15:33:20.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Documentation
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) astobj.h3.doxygen.diff.txt
( 1) astobj.h4.doxygen.diff.txt
( 2) astobj.h5.doxygen.diff.txt
Description:Attached is a documented astobj.h file.  I have never used astobj.h so you may want to review the docs for accuracy.

****** ADDITIONAL INFORMATION ******

Disclaimer on file.
Comments:By: Olle Johansson (oej) 2005-05-03 14:48:33

Great, great, great! Thank you for contributing to doxygen documentation! It's so important that we get the internal API:s well documented.

I'm sorry that I can't help you correcting it, since I haven't got the insights into astobj. But thank you anyway for a good contribution and remembering to mentioning your disclaimer :-)

By: Matthew Nicholson (mnicholson) 2005-05-03 17:18:38

Well as I write my C++ bindings more docs will come.  And then I will be able to write asterisk modules in C++ :)

By: ckruetze (ckruetze) 2005-05-03 17:35:02

Thanks for the excelent documentation.

One small note, in your patch you always add /*! \brief except for the last one where you missed out a space it is +/*!\brief.
I don't know if the space is necessary for doxygen, can't test it right now, but I thought I mention it anyway.

By: Matthew Nicholson (mnicholson) 2005-05-03 18:37:14

Nope, you don't need the space, but I added one any way.  I will wait for more comments before posting another patch.

By: Kevin P. Fleming (kpfleming) 2005-05-03 20:05:34

Some notes:

- the sample code has a call to _DESTROYALL but doesn't specify the name of a function to be used to actually destroy the objects

- the notes regarding read-locking and write-locking are incorrect; we are currently using the same type of lock for both purposes, and any future versions of astobj.h will continue to support read-lock -> write-lock upgrades

- the 'hashes' argument to ASTOBJ_COMPONENTS actually defines the number of containers that an object can be present in, regardless of whether they are list-based or hash-based

- there are some wording problems and typos.. the most common is 'and' instead of 'an' :-)

- for ASTOBJ_CONTAINER_UNLINK (and FIND_UNLINK), it is important to explicitly state that the caller is given the container's reference to the object, not a new reference (unlink FIND). This means that if the container had the last reference to the object, when the macro/function returns and UNREF is subsequently called on the object, the object will be destroyed (unless more references have been made before that time, of course)

- the 'brief' for FIND_UNLINK_FULL is the same as for FIND, instead of a combination of FIND and UNLINK (i.e. it doesn't say anything about the unlink part)

Unfortunately much of this will be changing when the new version of ASTOBJ is ready for public consumption, but I'm glad someone else has taken the time to try to understand what is there, it will be one more person who can help review the new version when the time comes :-)

By: Matthew Nicholson (mnicholson) 2005-05-04 08:46:58

Ok, ill go ahead and make these changes.  Is the new version in C++ :)?

By: Matthew Nicholson (mnicholson) 2005-05-04 09:19:42

I don't understand how the locking works.  It appears they are using the same lock for reading and writing, so wouldn't locking both of them cause a deadlock?

Also I don't quite get the hashes argument.  It only appears to have an effect when the hashes model is in use (I have made the changes either way).

Also the reason for the lack of destructors was they were not required by the example, but I have updated it to use them.

By: Kevin P. Fleming (kpfleming) 2005-05-04 12:04:58

The reason there are RDLOCK and WRLOCK macros is so that developers will use them to signal their intent to the astobj core; at some point in the future when we actually use underlying locks that have 'read' and 'write' modes, then everyone will benefit. Regardless, whatever implementation is in place will always support taking a RDLOCK on an object and then taking (upgrading to) a WRLOCK on the same object; the subsequent UNLOCK will do the right thing as well.

The 'hashes' argument determines the number of linking elements embedded in the object, for use by containers it may be placed in. An object defined with two 'hashes' can be linked into two separate containers simultaneously (this is why the container definition has a 'hash number' field, so it knows which array element inside the object it is supposed to use). I don't care for this implementation at all, any my in-progress rework doesn't work this way, but for the time being we need to document what is present in the tree.

By: Matthew Nicholson (mnicholson) 2005-05-04 12:25:33

Try this one.

By: Kevin P. Fleming (kpfleming) 2005-05-04 12:55:45

Committed to CVS HEAD, thanks!

By: Matthew Nicholson (mnicholson) 2005-05-04 15:38:15

One more change for the UNLINK set of macros.

By: Russell Bryant (russell) 2005-05-10 23:34:51

astobj is not in 1.0

By: Digium Subversion (svnbot) 2008-01-15 15:33:16.000-0600

Repository: asterisk
Revision: 5575

U   trunk/include/asterisk/astobj.h

------------------------------------------------------------------------
r5575 | kpfleming | 2008-01-15 15:33:15 -0600 (Tue, 15 Jan 2008) | 2 lines

add doxygen docs for astobj.h (bug ASTERISK-4047, with minor mods)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5575

By: Digium Subversion (svnbot) 2008-01-15 15:33:20.000-0600

Repository: asterisk
Revision: 5580

U   trunk/include/asterisk/astobj.h

------------------------------------------------------------------------
r5580 | markster | 2008-01-15 15:33:20 -0600 (Tue, 15 Jan 2008) | 2 lines

Fix minor doc issue (bug ASTERISK-4047)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5580