[ovs-dev] [InBandOVSDB 3/4] socket-util: Factor out new function inet_parse_active().

Justin Pettit jpettit at nicira.com
Mon Apr 26 18:28:50 UTC 2010


On Apr 26, 2010, at 10:38 AM, Ben Pfaff wrote:

> On Mon, Apr 26, 2010 at 12:02:34AM -0700, Justin Pettit wrote:
>> On Apr 20, 2010, at 4:37 PM, Ben Pfaff wrote:
>> 
>>> +/* Parses 'target', which should be a string in the format "<host>[:<port>]".
>>> + * <host> is required.  If 'default_port' is nonzero then <port> is optional
>>> + * and defaults to 'default_port'.
>>> *
>>> + * On success, returns true and stores the parsed remote address into '*sinp'.
>>> + * On failure, logs an error and returns false. */
>>> +bool
>>> +inet_parse_active(const char *target_, uint16_t default_port,
>>> +                  struct sockaddr_in *sinp)
>> 
>> I believe the description should refer to "target_" instead of
>> "target".  There are a few existing functions that have this same
>> issue in the file.
> 
> That's a style I use.  You're supposed to pretend that the _ isn't
> there.  (Seriously.)

Hmm.  That seems like an odd convention.  Is it commonly used?  The reason I noticed it initially was that I read the function's description and then saw a "free(target)" at the end of the function, which indicated to me that the caller gives up control of the argument.  Only when I read a bit more carefully, did I notice that the function's description and argument were different.

--Justin






More information about the dev mailing list