Re: CVS commit: src/include

[Jason Thorpe <thorpej%shagadelic.org@localhost>]

On Feb 18, 2006, at 6:53 AM, Elad Efrat wrote:

> Jason Thorpe wrote:
>
> > This looks okay to me.  Although, I would still do:
> >
> > typedef struct pw_policy *pw_policy_t;
> >
> > and return/pass a pw_policy_t.
>
> see attached diff.

This looks good.

> -e.
>
> --  
> Elad Efrat
> Index: include/util.h
> ===================================================================
> RCS file: /cvsroot/src/include/util.h,v
> retrieving revision 1.39
> diff -u -p -r1.39 util.h
> --- include/util.h      18 Feb 2006 10:53:33 -0000      1.39
> +++ include/util.h      18 Feb 2006 14:52:16 -0000
> @@ -56,9 +56,6 @@
>  #define        PW_POLICY_BYPASSWD      1
>  #define        PW_POLICY_BYGROUP       2
>
> -#define PW_POLICY_INIT { -1, -1, -1, -1, -1, -1, -1, \
> -                         -1, -1, -1, -1, -1, -1, -1 }
> -
>  __BEGIN_DECLS
>  struct disklabel;
>  struct iovec;
> @@ -68,22 +65,7 @@ struct utmp;
>  struct winsize;
>  struct sockaddr;
>
> -struct pw_policy {
> -       int32_t minlen;
> -       int32_t maxlen;
> -       int32_t minupper;
> -       int32_t maxupper;
> -       int32_t minlower;
> -       int32_t maxlower;
> -       int32_t mindigits;
> -       int32_t maxdigits;
> -       int32_t minpunct;
> -       int32_t maxpunct;
> -       int32_t mintoggles;
> -       int32_t maxtoggles;
> -       int32_t minclasses;
> -       int32_t maxclasses;
> -};
> +typedef struct pw_policy *pw_policy_t;
>
>  pid_t          forkpty(int *, char *, struct termios *, struct winsize *);
>  const char     *getbootfile(void);
> @@ -118,8 +100,9 @@ const char     *pw_getprefix(void);
>  void           pw_init(void);
>  int            pw_lock(int);
>  int            pw_mkdb(const char *, int);
> -int            pw_policy_load(struct pw_policy *, void *, int);
> -int            pw_policy_test(struct pw_policy *, char *);
> +pw_policy_t    pw_policy_load(void *, int);
> +int            pw_policy_test(pw_policy_t, char *);
> +void           pw_policy_free(pw_policy_t);
>  void           pw_prompt(void);
>  int            pw_setprefix(const char *);
>  int            secure_path(const char *);
> Index: lib/libutil/pw_policy.c
> ===================================================================
> RCS file: /cvsroot/src/lib/libutil/pw_policy.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 pw_policy.c
> --- lib/libutil/pw_policy.c     18 Feb 2006 10:52:48 -0000      1.3
> +++ lib/libutil/pw_policy.c     18 Feb 2006 14:52:16 -0000
> @@ -48,8 +48,8 @@
>                                  (min > 0 && min > n) ||          \
>                                  (max > 0 && max < n)
>
> -#define        HANDLER_PROTO   struct pw_policy *, int, char *, void *, void *
> -#define	HANDLER_ARGS	struct pw_policy *policy, int flag, char *pw,  
> void *arg, void *arg2
> +#define        HANDLER_PROTO   pw_policy_t, int, char *, void *, void *
> +#define	HANDLER_ARGS	pw_policy_t policy, int flag, char *pw, void  
> *arg, void *arg2
>
>  static int pw_policy_parse_num(char *, int32_t *);
>  static int pw_policy_parse_range(char *, int32_t *, int32_t *);
> @@ -59,6 +59,23 @@ static int pw_policy_handle_charclass(HA
>  static int pw_policy_handle_nclasses(HANDLER_PROTO);
>  static int pw_policy_handle_ntoggles(HANDLER_PROTO);
>
> +struct pw_policy {
> +       int32_t minlen;
> +       int32_t maxlen;
> +       int32_t minupper;
> +       int32_t maxupper;
> +       int32_t minlower;
> +       int32_t maxlower;
> +       int32_t mindigits;
> +       int32_t maxdigits;
> +       int32_t minpunct;
> +       int32_t maxpunct;
> +       int32_t mintoggles;
> +       int32_t maxtoggles;
> +       int32_t minclasses;
> +       int32_t maxclasses;
> +};
> +
>  struct pw_policy_handler {
>         const char *name;
>         int (*handler)(HANDLER_PROTO);
> @@ -322,18 +339,19 @@ pw_policy_handle_ntoggles(HANDLER_ARGS)
>         return (0);
>  }
>
> -int
> -pw_policy_load(struct pw_policy *policy, void *key, int how)
> +pw_policy_t
> +pw_policy_load(void *key, int how)
>  {
> +       pw_policy_t policy;
>         struct pw_policy_handler *hp;
>         char buf[BUFSIZ];
>
>         /* If there's no /etc/passwd.conf, don't touch the policy. */
> -       if (access(_PATH_PASSWD_CONF, R_OK) == -1)
> -               return (ENOENT);
> +       if (access(_PATH_PASSWD_CONF, R_OK) == -1) {
> +               errno = ENOENT;
>
> -       if (policy == NULL)
> -               return (EFAULT);
> +               return (NULL);
> +       }
>
>         /* No key provided. Use default. */
>         if (key == NULL) {
> @@ -399,10 +417,18 @@ pw_policy_load(struct pw_policy *policy,
>                  * Fail the policy because we don't know how to parse the
>                  * key we were passed.
>                  */
> -               return (EINVAL);
> +               errno = EINVAL;
> +
> +               return (NULL);
>         }
>
>   load_policies:
> +       policy = malloc(sizeof(struct pw_policy));
> +       if (policy == NULL)
> +               return (NULL);
> +
> +       memset(policy, 0, sizeof(struct pw_policy));
> +
>         hp = &handlers[0];
>         while (hp->name != NULL) {
>                 int error;
> @@ -412,18 +438,20 @@ pw_policy_load(struct pw_policy *policy,
>                 if (*buf) {
>                         error = hp->handler(policy, LOAD_POLICY, NULL, buf,
>                                             hp->arg2);
> -                       if (error)
> -                               return (error);
> +                       if (error) {
> +                               errno = error;
> +                               return (NULL);
> +                       }
>                 }
>
>                 hp++;
>         }
>
> -       return (0);
> +       return (policy);
>  }
>
>  int
> -pw_policy_test(struct pw_policy *policy, char *pw)
> +pw_policy_test(pw_policy_t policy, char *pw)
>  {
>         struct pw_policy_handler *hp;
>
> @@ -443,3 +471,9 @@ pw_policy_test(struct pw_policy *policy,
>
>         return (0);
>  }
> +
> +void
> +pw_policy_free(pw_policy_t policy)
> +{
> +       free(policy);
> +}

-- thorpej