[SSSD] Make return codes of basic sysdb operations consistent

Jakub Hrozek jhrozek at redhat.com
Mon Apr 28 19:06:55 UTC 2014


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:
> > > >> Hello,
> > > >> 
> > > >> I recently started working on ticket #1991.
> > > >> https://fedorahosted.org/sssd/ticket/1991
> > > >> 
> > > >> "The return codes of various sysdb operations differ. Some search
> > > >> operations would return ENOENT if they don't find a matching object some
> > > >> would return EOK but an empty result list."
> > > >> 
> > > >> I think that the core of the problem is the expectation that ldb_search
> > > >> returns LDB_ERR_NO_SUCH_OBJECT in case no results were found. But it
> > > >> instead returns LDB_SUCCESS and sets result->count to 0.
> > > >> 
> > > >> 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. 
> > > >> 
> > > >> Thanks for opinions.
> > > >> 
> > > >> Pavel Reichl
> > > >
> > > >I agree with returning an error code rather than an empty list. The
> > > >empty list has bitten us in the past already, the return code would make
> > > >sure the programmer has to explicitly handle it.
> > > >
> > > >I wonder if we could go even further and introduce a new error code
> > > New error code will make sense if you want more precise expression of error.
> > > 
> > > >(ERR_NO_SUCH_OBJECT). ENOENT is used quite a bit in the code, so the
> > > What will be difference between ENOENT and ERR_NO_SUCH_OBJECT?
> > >     for example: ERR_MISSING_CONF
> > 
> > 1) strerror would report better error string
> > 2) ENOENT can be returned by other underlying function (and therefore
> > could be treated as a real error) , ERR_NO_SUCH_OBJECT would only be
> > returned by sysdb.
> 
> So are we really going with ERR_NO_SUCH_OBJECT? I don't say it's a bad
> idea. I'm just afraid it may be too much work. 
> 
> If we'll just make sure that ENOENT is returned if no result is found we
> have to change just the implementation of some sysdb functions and in
> code that calls them.  
> 
> But if we go with new return value we'll have to change implementation
> of ALL (relevant) sysdb functions and *ALL* code that is calling them.
> It may prove difficult to trace the 'journey' of ENOENT return value
> from sysdb functions through calling hierarchy. I think that this would
> create a potential for introduction many new bugs.

Yeah, you might be right...I haven't done any prototyping, I was just
assuming that error codes from sysdb functions never bubble up but are
dealt with by the caller. I guess you know the problem space better
since you already spent some time with the sysdb code lately.

Anyway, even if we were changing the errno codes, it would be a separate
patchset. 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.

But in general, I think the specific error codes make sense, especially
when dealing with code that is higher up than system level..for instance
in the DBus responder, (ab)using the standard errno codes only leads to
code full of EIOs and EFAULTs so there I'm trying to use new error codes
where possible..



More information about the sssd-devel mailing list