[ovs-dev] [QoS v2 12/17] netlink: Add support for Netlink table dumping.

Ben Pfaff blp at nicira.com
Fri Jun 11 16:27:19 UTC 2010


On Wed, Jun 09, 2010 at 03:09:43PM -0700, Justin Pettit wrote:
> On Jun 8, 2010, at 1:41 PM, Ben Pfaff wrote:
> 
> > +/* Attempts to retrieve another reply from 'dump', which must have been
> > + * initialized with nl_dump_start().  If successful, returns true and points
> > + * 'reply->data' and 'reply->size' to the message that was retrieved.  The
> > + * caller must not modify 'reply' (because it points into the middle of a
> > + * larger buffer).  On failure, returns false.
> 
> The way this is written, it's not clear what the caller is supposed to
> do with 'reply' on failure.  Looking at the code, it appears that the
> caller must call nl_dump_done() regardless of the result to prevent a
> memory leak.  I think just moving that sentence that begins with "The
> caller must..." to the end of that paragraph would be sufficient.

The caller can't really do anything with 'reply' on failure, because the
function sets reply->data to NULL and reply->size to 0 in that case.  

I changed the comment to this in my working copy.  I hope that it is
clearer:

/* Attempts to retrieve another reply from 'dump', which must have been
 * initialized with nl_dump_start().
 *
 * If successful, returns true and points 'reply->data' and 'reply->size' to
 * the message that was retrieved.  The caller must not modify 'reply' (because
 * it points into the middle of a larger buffer).
 *
 * On failure, returns false and sets 'reply->data' to NULL and 'reply->size'
 * to 0.  Failure might indicate an actual error or merely the end of replies.
 * An error status for the entire dump operation is provided when it is
 * completed by calling nl_dump_done().
 */




More information about the dev mailing list