[SSSD] Make return codes of basic sysdb operations consistent

Lukas Slebodnik lslebodn at redhat.com
Wed May 28 08:16:28 UTC 2014


On (28/05/14 10:14), Jakub Hrozek wrote:
>On Wed, May 28, 2014 at 09:31:28AM +0200, Pavel Reichl wrote:
>> On Mon, 2014-04-28 at 21:06 +0200, Jakub Hrozek wrote:
>> > On Mon, Apr 28, 2014 at 02:31:36PM +0200, Pavel Reichl wrote:
>> > > On Tue, 2014-04-22 at 13:09 +0200, Jakub Hrozek wrote:
>> > > > On Tue, Apr 22, 2014 at 12:31:36PM +0200, Lukas Slebodnik wrote:
>> > > > > On (22/04/14 11:39), Jakub Hrozek wrote:
>> > > > > >On Tue, Apr 22, 2014 at 11:31:31AM +0200, Pavel Reichl wrote:
>> [snip]
>> > > > > >> I think it would be best for sysdb functions in case that no results
>> > > > > >> were found to return both ENOENT value and 'properly' empty list. 
>> [snip]
>> > So I'd say we should go ahead with the current plan, change
>> > only returning ENOENT vs EOK+empty set and worry about errno codes
>> > later.
>> 
>> OK, so if ENOENT is returned do we guarantee that output parameters are
>> valid and 'output count' is set to 0?
>
>I'm not concerned about separate count, that's usually not the issue.
>
>Looking at the sysdb API, we seem to be returning either ldb_message or
>ldb_result. I think the original issue was that we could have returned EOK
>but either let the pointer to ldb_message undefined or NULL, I'm not sure
>which it was.
>
>Then we had crashes in code that looked perfectly valid, like:
>
>    struct ldb_message *msg;
>
>    ret = sysdb_search(sysdb, filter, &msg);
>    if (ret != EOK) goto fail;
>
>    for (int i = 0; i < msg->num_elements; i++) {
>        do_stuff();
>    }
>
>Dereferencing msg was the problem.
>
>And personally I don't think we should guarantee setting msg to anything
>on return code != EOK. Just let the caller handle the return code.
+1

LS



More information about the sssd-devel mailing list