Re: [PATCH] libxl: Be more careful with error handling in libxl__dm_runas_helper()

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2670 bytes --]

On Wed, 2015-11-25 at 16:56 -0500, Boris Ostrovsky wrote:
> getpwnam_r() has fairly complicated return rules. From man pages:
> 
>   RETURN VALUE
>       ...
>       On success, getpwnam_r() and getpwuid_r() return zero, and set
>       *result to pwd.  If no matching  password record was found,
> these
>       functions return 0 and store NULL in *result. In case of error,
>       an error number is returned, and NULL is stored in *result.
>   ERRORS
>       0 or ENOENT or ESRCH or EBADF or EPERM or ...
>             The given name or uid was not found.
> 
Wow! Given how much he likes error handling, I can't wait to see Ian
Jackson jumping on this! :-PP

> While it's not clear what ellipses are meant to be, the way we
> currently
> treat return values from getpwnam_r() is no sufficient. In fact, two
> of
> my systems behave differently when username is not found: one returns
> ENOENT and the other returns 0. Both set *result to NULL.
> 
Nice. Or not. :-/

Anyway, given exactly those ellipses, wouldn't it be safer to go the
other way round? I mean something like:

> @@ -740,12 +740,17 @@ static int libxl__dm_runas_helper(libxl__gc
> *gc, const char *username)
>          ret = getpwnam_r(username, &pwd, buf, buf_size, &user);
>          if (ret == ERANGE) {
>              buf_size += 128;
> +            if (retry_cnt++ > 10)
> +                    return ERROR_FAIL;
>              continue;
>          }
> -        if (ret != 0)
> -            return ERROR_FAIL;
> -        if (user != NULL)
> -            return 1;
> +        if (user == NULL) {
> +            if (!ret || (ret == ENOENT) || (ret == ESRCH) ||
> +                (ret == EBADF) || (ret == EPERM))
> +                return ERROR_NOTFOUND;
> +            else
> +                return ERROR_FAIL;
> +        }
>
  if (user == NULL) {
      if (ret == EINTR || ret == EIO || ret == EMFILE ||
          ret == ENFILE || ret == ENOMEM)
          return ERROR_FAIL;
      else
          return ERROR_NOTFOUND;
  }

Also, considering libxl attitude toward out-of-memory errors, should we
deal with ENOMEM in a special way?

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel