[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] libxl: Be more careful with error handling in libxl__dm_runas_helper()
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:
signature.asc _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |