[SSSD] [PATCH] Consolidate code for splitting strings by separator

Stephen Gallagher sgallagh at redhat.com
Thu Dec 10 17:46:38 UTC 2009


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/09/2009 07:08 PM, Jakub Hrozek wrote:
> There were two functions for parsing strings by a separator. This patch
> consolidates on the one previously used in confdb. This also allows
> stripping the tokens of whitespace.
> 
> Fixes: #319


Nack.

strpbrk()'s stopset value is not really a string, it's a character
array. The difference is that it will stop on ANY character listed in
that string, not just the first one. I'm aware that at present we are
only using single-character separators, but this will bite us eventually.

I'd rather that we just defined the separator as a const char *sep in
the function arguments and then just allocated a one-char array to pass
into strpbrk() as the stopset. There will be no ambiguity this way.

If someday later we need multiple-character separators, we can add a new
function that handles this properly.

Also, space and tab aren't technically the only possible whitespace
characters. We should probably handle carriage return ('\n') and line
feed ('\r') characters as whitespace as well, just in case the passed-in
string is multi-line.



Please initialize list = NULL. Otherwise, if talloc_realloc() returns
NULL on the first pass through the loop, the talloc_free() will be
operating on random data.



While you're in there, would you mind adding a few comments? Digesting
that function was a bit painful.


- -- 
Stephen Gallagher
RHCE 804006346421761

Delivering value year after year.
Red Hat ranks #1 in value among software vendors.
http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkshM/4ACgkQeiVVYja6o6OmxwCeJqBd6kKKtGrfACLjHmhu8n2h
UgQAn0K03ReOuAumYNyuByTsEJ8c/Rr0
=r4Nx
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list