[libvirt] [PATCHv2 1/7] util: Fix docs for virBitmapParse

[Martin Kletzander]

On 01/23/2013 12:18 AM, Peter Krempa wrote:
> On 01/22/13 23:25, Eric Blake wrote:
>> On 01/22/2013 02:30 PM, Peter Krempa wrote:
>>> The documentation comment virBitmapParse didn't document the @sep
>>> parameter.
>>> ---
>>>   src/util/virbitmap.c | 16 +++++++++-------
>>>   1 file changed, 9 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
>>> index ca82d1b..e3ca4ff 100644
>>> --- a/src/util/virbitmap.c
>>> +++ b/src/util/virbitmap.c
>>> @@ -265,23 +265,25 @@ char *virBitmapFormat(virBitmapPtr bitmap)
>>>   /**
>>>    * virBitmapParse:
>>>    * @str: points to a string representing a human-readable bitmap
>>> + * @sep: separator character
>>>    * @bitmap: a bitmap created from @str
>>>    * @bitmapSize: the upper limit of num of bits in created bitmap
>>>    *
>>>    * This function is the counterpart of virBitmapFormat. This
>>> function creates
>>>    * a bitmap, in which bits are set according to the content of @str.
>>>    *
>>> - * @str is a comma separated string of fields N, which means a
>>> number of bit
>>> - * to set, and ^N, which means to unset the bit, and N-M for ranges
>>> of bits
>>> - * to set.
>>> + * @str is a string of fields N separated by a character @sep, which
>>> means a
>>> + * number of bit to set, and ^N, which means to unset the bit, and
>>> N-M for
>>> + * ranges of bits to set.
>>
>> This wording confused me.  @sep isn't between each field, so much as at
>> the end of all of the fields.  Most callers pass 0, but there are some
>> callers that pass ',' and even one that passes 'n'; so the use of 'sep'
>> is when you are parsing a bitmap out of a larger string, and want to
>> stop at the separator that says where the bitmap ends and the rest of
>> the string continues (and NOT that it separates fields, unless 'sep'
>> happens to be ',' to parse exactly one field).
>>
>> But I don't know if I have any better wording, so weak ACK, especially
>> if you can improve it.
> 
> Uhm, now that you pointed out I realized that I don't really know what
> is the argument used for. I will need to think about it a little more.
> If it's used for separating the bitmap from a larger string I will need
> to tweak the comment. I thought it's used to allow other separators than
> commas in the bitmap that is being parsed itself. I honestly didn't read
> the code carefully enough.
> 
> I'll try to reverse engineer it in the morning.
> 

>From what I knew, the @sep is used as a character that should not be
parsed, delimiter let's say.  You can see it's being checked for
occurrence whenever the check is made for '\0'.  I saw it in another
function associated with file loading, which I can't find even though I
looked throughout the whole libvirt codebase.

I'd suggest changing it to delim(iter).  And if I'll find the other part
where it wasn't documented as well, I'll change it there too.

Martin