[SSSD] [PATCH] Fix error messages in tools

Stephen Gallagher sgallagh at redhat.com
Tue Oct 6 18:29:52 UTC 2009


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

On 10/06/2009 07:40 AM, Stephen Gallagher wrote:
> On 10/06/2009 06:32 AM, Jakub Hrozek wrote:
>> On 10/05/2009 07:43 PM, Stephen Gallagher wrote:
>>> Replied to the wrong patch earlier...
> 
>>> Nack.
>>> sss_getpwnam_done() and friends should return EIO if the sysdb replies
>>> with more than one result, I think. EINVAL implies that the fault was
>>> with the caller, whereas EIO shows us that something went wrong internally.
> 
> 
>> Fixed in getpwnam(), in getgrnam() there is no such check as it also
>> returns the members of the group.
> 
>>> I don't like the gratuitous use of discard_const_p in
>>> sss_getpwnam_done(). These sync ops are theoretically useful in other
>>> places in our code. I don't want people assuming that they can modify
>>> the contents of the res->data->name field (for example). They should be
>>> talloc_strdup()-ed into the res structure.
> 
> 
>> True, talloc_strdup() is used now.
> 
>>> sss_useradd.c:
>>> ERROR("A user or group with the same name or UID already exists\n");
>>> should be
>>> ERROR("A user or group with the same name or ID already exists\n");
> 
> 
>> Fixed.
> 
>>> tools_util.c:
>>> If enumerate=true, shouldn't we just make a call to something like a
>>> sysdb_getgrent_sync and compare the resulting complete list with the
>>> received groups instead of making N complete sync calls, where N could
>>> be equal to (or greater than) the total list of groups in the DB?
> 
>>> For that matter, since this is the local domain, it might be worth
>>> enumerating them anyway, even if enumerations would normally not happen,
>>> since local performance won't be a problem.
> 
>> This is not addressed. I discussed this (very briefly) with Simo, we
>> agreed that we should stick to sysdb_getgrnam() for now, because as long
>> as our searches match indexes (they do, we search based on name) the
>> search for a single group is pretty fast.
> 
>> 	Jakub
> 
> It all looks good to me.
> 
> Ack.
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel
> 
> 

Nack after all. Jenny noticed that there are some places where the
returned error code should be nonzero and it is not.

Please fix these up and resubmit.
_______________________________________________
sssd-devel mailing list
sssd-devel at lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel

- -- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkrLjJsACgkQeiVVYja6o6NaVQCffuE44i/VWm/c13C1VxIr+0VX
CwUAoIlmiqPNkFepccHYKr1dihwO2n/F
=/3dk
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list