[libvirt] [PATCH] util: distinguish get{pw|gr}nam_r error from the "not found" cases
manpage: "If no matching 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." So we can distinguish function error from "no found" cases based on the return value. --- src/util/virutil.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index 4605c78..9a4682e 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -2532,6 +2532,7 @@ virGetUserIDByName(const char *name, uid_t *uid) goto cleanup; } + errno = 0; while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) { virReportOOMError(); @@ -2540,16 +2541,11 @@ virGetUserIDByName(const char *name, uid_t *uid) } if (!pw) { - if (rc != 0) { - char buf[1024]; - /* log the possible error from getpwnam_r. Unfortunately error - * reporting from this function is bad and we can't really - * rely on it, so we just report that the user wasn't found */ - VIR_WARN("User record for user '%s' was not found: %s", - name, virStrerror(rc, buf, sizeof(buf))); - } - - ret = 1; + if (rc == 0) + ret = 1; + else + virReportSystemError(errno = rc, + _("cannot getpwnam_r(%s)"), name); goto cleanup; } @@ -2577,8 +2573,13 @@ virGetUserID(const char *user, uid_t *uid) user++; } else { int rc = virGetUserIDByName(user, uid); - if (rc <= 0) + if (rc > 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("User '%s' was not found"), user); + return -1; + } else { return rc; + } } if (virStrToLong_ui(user, NULL, 10, &uint_uid) < 0 || @@ -2616,6 +2617,7 @@ virGetGroupIDByName(const char *name, gid_t *gid) goto cleanup; } + errno = 0; while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) { virReportOOMError(); @@ -2624,16 +2626,11 @@ virGetGroupIDByName(const char *name, gid_t *gid) } if (!gr) { - if (rc != 0) { - char buf[1024]; - /* log the possible error from getgrnam_r. Unfortunately error - * reporting from this function is bad and we can't really - * rely on it, so we just report that the user wasn't found */ - VIR_WARN("Group record for user '%s' was not found: %s", - name, virStrerror(rc, buf, sizeof(buf))); - } - - ret = 1; + if (rc == 0) + ret = 1; + else + virReportSystemError(errno = rc, + _("cannot getgrnam_r(%s)"), name); goto cleanup; } @@ -2661,8 +2658,13 @@ virGetGroupID(const char *group, gid_t *gid) group++; } else { int rc = virGetGroupIDByName(group, gid); - if (rc <= 0) + if (rc > 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("Group '%s' was not found"), group); + return -1; + } else { return rc; + } } if (virStrToLong_ui(group, NULL, 10, &uint_gid) < 0 || -- 1.7.11.2
On 03/07/13 17:08, Guannan Ren wrote:
manpage: "If no matching 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."
So we can distinguish function error from "no found" cases based on the return value. --- src/util/virutil.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-)
NACK, we already tried that, but getpwnam_r sucks and there's no reasonable replacement. See http://www.redhat.com/archives/libvir-list/2012-December/msg00207.html for the full reasons we stick with the ugly code. Peter
On 03/08/2013 12:10 AM, Peter Krempa wrote:
On 03/07/13 17:08, Guannan Ren wrote:
manpage: "If no matching 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."
So we can distinguish function error from "no found" cases based on the return value. --- src/util/virutil.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-)
NACK, we already tried that, but getpwnam_r sucks and there's no reasonable replacement.
See http://www.redhat.com/archives/libvir-list/2012-December/msg00207.html for the full reasons we stick with the ugly code.
Peter
Accept the nack :)
participants (2)
-
Guannan Ren -
Peter Krempa