On Mon, Oct 08, 2012 at 02:08:59PM +0200, Peter Krempa wrote:
> On 10/06/12 04:52, Marcelo Cerri wrote:
> >This patch updates virGetUserID and virGetGroupID to be able to parse a
> >user or group name in a similar way to coreutils' chown. This means that
> >a numeric value with a leading plus sign is always parsed as an ID,
> >otherwise the functions try to parse the input first as a user or group
> >name and if this fails they try to parse it as an ID.
> >---
> > src/util/util.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> > 1 file changed, 74 insertions(+), 13 deletions(-)
> >
> >diff --git a/src/util/util.c b/src/util/util.c
> >index 43fdaf1..694ed3d 100644
> >--- a/src/util/util.c
> >+++ b/src/util/util.c
> >@@ -2495,9 +2495,11 @@ char *virGetGroupName(gid_t gid)
> > return virGetGroupEnt(gid);
> > }
> >
> >-
> >-int virGetUserID(const char *name,
> >- uid_t *uid)
> >+/* Search in the password database for a user id that matches the user name
> >+ * `name`. Returns 0 on success, -1 on a critical failure or 1 if name cannot
> >+ * be parsed.
> >+ */
> >+static int virGetUserIDByName(const char *name, uid_t *uid)
> > {
> > char *strbuf;
> > struct passwd pwbuf;
> >@@ -2530,11 +2532,10 @@ int virGetUserID(const char *name,
> > }
> > }
> > if (rc != 0 || pw == NULL) {
> >- virReportSystemError(rc,
> >- _("Failed to find user record for name '%s'"),
> >- name);
> >+ VIR_DEBUG("Failed to find user record for user '%s' (error = %d)",
> >+ name, rc);
>
> Although this will work most of the times when an error occurs it
> will be masked as if the user wasn't found. Unfortunately getpwuid_r
> and friends have very bad error reporting. The only effective way to
> distinguish errors from non-existent user entries (according to the
> manpage) is to check set errno before the call and check it
> afterwards.
Do you think that I should keep virReportSystemError instead of
VIR_DEBUG here?
>
> > VIR_FREE(strbuf);
> >- return -1;
> >+ return 1;
> > }
> >
> > *uid = pw->pw_uid;
> >@@ -2544,9 +2545,41 @@ int virGetUserID(const char *name,
> > return 0;
> > }
> >
> >+/* Try to match a user id based on `user`. The default behavior is to parse
> >+ * `user` first as a user name and then as a user id. However if `user`
> >+ * contains a leading '+', the rest of the string is always parsed as a uid.
> >+ *
> >+ * Returns 0 on success and -1 otherwise.
> >+ */
> >+int virGetUserID(const char *user, uid_t *uid)
> >+{
> >+ unsigned int uint_uid;
> >
> >-int virGetGroupID(const char *name,
> >- gid_t *gid)
> >+ if (*user == '+') {
> >+ user++;
> >+ } else {
> >+ int rc = virGetUserIDByName(user, uid);
> >+ if (rc <= 0)
> >+ return rc;
> >+ }
> >+
> >+ if (virStrToLong_ui(user, NULL, 10, &uint_uid) < 0 ||
> >+ ((uid_t) uint_uid) != uint_uid) {
> >+ virReportError(VIR_ERR_INVALID_ARG, _("Failed to parse user '%s'"),
> >+ user);
> >+ return -1;
> >+ }
> >+
> >+ *uid = uint_uid;
> >+
> >+ return 0;
> >+}
> >+
>
> Otherwise looks ok, so ACK. I'll push this after I get a review on
> the followup patch that fixes the issue with errors being masked
> while getting the user entry.
>
> Peter
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list