[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]