-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
>> This is the last of the non ELAPI patche that I have in my
repo.
>
>
> The only functional problem I see is that the implementation does not
> strip leading whitespace, i.e. " foo, bar " would become { "
foo", "bar"
> }. Otherwise seems to work fine. Some comments below:
>
The leading space at the beginning of the value is stripped when value
is read from
key = value
line in the INI file.
And then the logic cleans the spaces around separator so we should be
fine here.
Ah, sorry, I was only looking at the individual function and missed that.
> Is there any reason to arbitrarily limit the number of
separators?
>
> Also, maybe it would be nicer to use strpbrk() instead of
> for(j = 0; j < lensep; j++) {
> if(buff[i] == locsep[j])
> ..one less loop.
>
I do not see a reason why we should not limit them.
strpbrk definitely has a loop inside so this is a wash.
I think that limited number of separators reminds developers not go
"wild" with the format.
IMO there should be one separator. 3 is a courtesy.
The down side of using strpbrk() is that it is not known which separator
actually matched.
Though this capability is not used I had it in mind for testing purposes.
Changing to strpbrk would cause me to rewrite the logic and retest it.
Do you think it is worth it?
If there's any functionality planned that involves the character that
matched, then no, I proposed strpbrk() mainly to get rid of the for()
and if().
> Instead of:
> while ((i < dlen) && (isspace(*start)))
> maybe you could use:
> while (*start && (isspace(*start)))
> as the data will be null-terminated strings anyway. That way you could
> avoid doing i++ before the loop and i-- immediatelly after it.
I need to adjust "i" anyways to skip the spaces and not process them
twice so I would have to save initial position and then get delta and
then adjust "i" using delta.
I think in general case you suggestion is correct but in this case it
makes sense to do it as written.
OK.
- -> ACK
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora -
http://enigmail.mozdev.org/
iEUEARECAAYFAktYf9AACgkQHsardTLnvCUVRgCcCTr/2wVOUlrkzvOPe5uUGam2
ew0Al3IyS1i8Wsk1Y/okVAeFjvryr6A=
=GcKq
-----END PGP SIGNATURE-----