Re: memory leak in dir-lookup found

[Neal H Walfield]

On Sun, Apr 01, 2001 at 05:47:39PM -0400, Roland McGrath wrote:
> > I have added several convenience function and revised the source
> > appropriately.  It also uses a new calling convention making
> > not checking for errors difficult, however, this is at the cost
> > of ABI incompatibility, however, I feel that this is worth it.
> 
> I don't think this is a useful interface change.  To my mind, your
> interface makes it easier to fail to check for errors and more dangerous to
> do so.  That is, you can just ignore the error_t return value and an
> undetected error results in an uninitialized result parameter that the
> careless code will then try to use with unpredictable results.  Conversely,
> with the interface that returns the pointer, a caller will never ignore the
> return value (because that's the only point of calling the function); a
> careless caller will then dereference a null pointer, which will crash
> quickly and clearly at that spot.

I disagree, if we set *USER to NULL on failure, we get the same effect.
Additionally, the user will be less likely to ignore the result, as they
can no longer do:

        err = func (..., iohelp_dup_iouser (cred->user), ...)
        if (err)
          ...

but at a minimum will be forced to do:

        iohelp_dup_iouser (&newuser, cred->user);
        err = func (..., newuser, ...);
        if (err)
          {
            ...
          }

And if they have gone this far, it is trivial to check the return value.

With:

        err = iohelp_dup_iouser (&newuser, cred->user);
        if (! err)
          err = func (..., newuser, ...);
        if (err)
          {
            if (newuser)
              iohelp_free_iouser (newuser);
            ...
          }


> When the only possible reason for
> failure is ENOMEM, there is no benefit in returning an error code.

iohelp_reauth can fail with other errors which, at the moment, we merely
discard.  Thus, in this instance, it is better to return an error_t.

pgpcSZSE1fScp.pgp
Description: PGP signature