Ted Unangst wrote:
> less has a peculiar estrdup function. unlike ecalloc etc., it only
> prints an error but doesn't quit. But the callers don't seem to check
> for null. And in many places they call a function called save()
> instead.
> 
> It is clearer to make estrdup() quit and use it everywhere.

ok mmcc@

That said, error()'s name very strongly implies that it quits, so we
should look into fixing that too.

> Index: cmdbuf.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/cmdbuf.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 cmdbuf.c
> --- cmdbuf.c  6 Nov 2015 15:50:33 -0000       1.9
> +++ cmdbuf.c  7 Nov 2015 04:04:50 -0000
> @@ -688,7 +688,7 @@ cmd_addhist(struct mlist *mlist, const c
>                * Save the command and put it at the end of the history list.
>                */
>               ml = ecalloc(1, sizeof (struct mlist));
> -             ml->string = save(cmd);
> +             ml->string = estrdup(cmd);
>               ml->next = mlist;
>               ml->prev = mlist->prev;
>               mlist->prev->next = ml;
> @@ -1207,7 +1207,7 @@ histfile_name(void)
>               if (strcmp(name, "-") == 0 || strcmp(name, "/dev/null") == 0)
>                       /* $LESSHISTFILE == "-" means don't use history file */
>                       return (NULL);
> -             return (save(name));
> +             return (estrdup(name));
>       }
>  
>       /* Otherwise, file is in $HOME if enabled. */
> Index: command.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/command.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 command.c
> --- command.c 6 Nov 2015 15:58:01 -0000       1.21
> +++ command.c 7 Nov 2015 04:04:50 -0000
> @@ -203,7 +203,7 @@ exec_mca(void)
>               if (*cbuf == '\0')
>                       every_first_cmd = NULL;
>               else
> -                     every_first_cmd = save(cbuf);
> +                     every_first_cmd = estrdup(cbuf);
>               break;
>       case A_OPT_TOGGLE:
>               toggle_option(curropt, opt_lower, cbuf, optflag);
> Index: decode.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/decode.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 decode.c
> --- decode.c  6 Nov 2015 15:58:01 -0000       1.11
> +++ decode.c  7 Nov 2015 04:04:50 -0000
> @@ -680,9 +680,9 @@ add_hometable(char *envname, char *def_f
>       PARG parg;
>  
>       if (envname != NULL && (filename = lgetenv(envname)) != NULL)
> -             filename = save(filename);
> +             filename = estrdup(filename);
>       else if (sysvar)
> -             filename = save(def_filename);
> +             filename = estrdup(def_filename);
>       else
>               filename = homefile(def_filename);
>       if (filename == NULL)
> Index: edit.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/edit.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 edit.c
> --- edit.c    6 Nov 2015 15:50:33 -0000       1.13
> +++ edit.c    7 Nov 2015 04:04:50 -0000
> @@ -232,7 +232,7 @@ edit_ifile(IFILE ifile)
>               return (0);
>       }
>  
> -     filename = save(get_filename(ifile));
> +     filename = estrdup(get_filename(ifile));
>       /*
>        * See if LESSOPEN specifies an "alternate" file to open.
>        */
> Index: filename.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/filename.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 filename.c
> --- filename.c        6 Nov 2015 15:50:33 -0000       1.18
> +++ filename.c        7 Nov 2015 04:04:50 -0000
> @@ -643,14 +643,14 @@ open_altfile(char *filename, int *pf, vo
>                       if (returnfd > 1 && status == 0) {
>                               *pfd = NULL;
>                               *pf = -1;
> -                             return (save(FAKE_EMPTYFILE));
> +                             return (estrdup(FAKE_EMPTYFILE));
>                       }
>                       return (NULL);
>               }
>               ch_ungetchar(c);
>               *pfd = (void *) fd;
>               *pf = f;
> -             return (save("-"));
> +             return (estrdup("-"));
>       }
>       cmd = readfd(fd);
>       pclose(fd);
> Index: funcs.h
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/funcs.h,v
> retrieving revision 1.10
> diff -u -p -r1.10 funcs.h
> --- funcs.h   6 Nov 2015 15:09:07 -0000       1.10
> +++ funcs.h   7 Nov 2015 04:04:50 -0000
> @@ -9,7 +9,6 @@
>  struct mlist;
>  struct loption;
>  
> -extern char *save(const char *);
>  extern void *ecalloc(int, unsigned int);
>  /*PRINTFLIKE1*/
>  extern char *easprintf(const char *, ...);
> Index: ifile.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/ifile.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 ifile.c
> --- ifile.c   6 Nov 2015 15:50:33 -0000       1.9
> +++ ifile.c   7 Nov 2015 04:04:50 -0000
> @@ -107,7 +107,7 @@ new_ifile(char *filename, struct ifile *
>        * Allocate and initialize structure.
>        */
>       p = ecalloc(1, sizeof (struct ifile));
> -     p->h_filename = save(filename);
> +     p->h_filename = estrdup(filename);
>       p->h_scrpos.pos = -1;
>       p->h_opened = 0;
>       p->h_hold = 0;
> Index: lsystem.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/lsystem.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 lsystem.c
> --- lsystem.c 6 Nov 2015 15:50:33 -0000       1.12
> +++ lsystem.c 7 Nov 2015 04:04:50 -0000
> @@ -85,7 +85,7 @@ lsystem(const char *cmd, const char *don
>       p = NULL;
>       if ((shell = lgetenv("SHELL")) != NULL && *shell != '\0') {
>               if (*cmd == '\0') {
> -                     p = save(shell);
> +                     p = estrdup(shell);
>               } else {
>                       char *esccmd = shell_quote(cmd);
>                       if (esccmd != NULL) {
> @@ -96,9 +96,9 @@ lsystem(const char *cmd, const char *don
>       }
>       if (p == NULL) {
>               if (*cmd == '\0')
> -                     p = save("sh");
> +                     p = estrdup("sh");
>               else
> -                     p = save(cmd);
> +                     p = estrdup(cmd);
>       }
>       (void) system(p);
>       free(p);
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/main.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 main.c
> --- main.c    6 Nov 2015 15:50:33 -0000       1.19
> +++ main.c    7 Nov 2015 04:04:50 -0000
> @@ -107,7 +107,7 @@ main(int argc, char *argv[])
>  
>       s = lgetenv(less_is_more ? "MORE" : "LESS");
>       if (s != NULL)
> -             scan_option(save(s));
> +             scan_option(estrdup(s));
>  
>       if (less_is_more) {
>               /* this is specified by XPG */
> @@ -257,20 +257,6 @@ main(int argc, char *argv[])
>  }
>  
>  /*
> - * Copy a string to a "safe" place
> - * (that is, to a buffer allocated by calloc).
> - */
> -char *
> -save(const char *s)
> -{
> -     char *p;
> -
> -     if ((p = estrdup(s)) == NULL)
> -             quit(QUIT_ERROR);
> -     return (p);
> -}
> -
> -/*
>   * Allocate memory.
>   * Like calloc(), but never returns an error (NULL).
>   */
> @@ -310,9 +296,12 @@ easprintf(const char *fmt, ...)
>  char *
>  estrdup(const char *str)
>  {
> -     char *n = strdup(str);
> -     if (n == NULL && str != NULL) {
> +     char *n;
> +     
> +     n = strdup(str);
> +     if (n == NULL) {
>               error("Cannot allocate memory", NULL_PARG);
> +             quit(QUIT_ERROR);
>       }
>       return (n);
>  }
> Index: optfunc.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/optfunc.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 optfunc.c
> --- optfunc.c 6 Nov 2015 15:50:33 -0000       1.11
> +++ optfunc.c 7 Nov 2015 04:04:50 -0000
> @@ -347,7 +347,7 @@ opt__P(int type, char *s)
>               default:   proto = &prproto[PR_SHORT];          break;
>               }
>               free(*proto);
> -             *proto = save(s);
> +             *proto = estrdup(s);
>               break;
>       case QUERY:
>               parg.p_string = prproto[pr_type];
> Index: option.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/option.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 option.c
> --- option.c  6 Nov 2015 15:50:33 -0000       1.11
> +++ option.c  7 Nov 2015 04:04:50 -0000
> @@ -151,7 +151,7 @@ scan_option(char *s)
>                       if (s == NULL)
>                               return;
>                       if (*str == '+')
> -                             every_first_cmd = save(str+1);
> +                             every_first_cmd = estrdup(str+1);
>                       else
>                               ungetsc(str);
>                       free(str);
> Index: prompt.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/prompt.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 prompt.c
> --- prompt.c  6 Nov 2015 15:50:33 -0000       1.15
> +++ prompt.c  7 Nov 2015 04:04:50 -0000
> @@ -75,12 +75,12 @@ static char *mp;
>  void
>  init_prompt(void)
>  {
> -     prproto[0] = save(s_proto);
> -     prproto[1] = save(less_is_more ? more_proto : m_proto);
> -     prproto[2] = save(less_is_more ? more_M_proto : M_proto);
> -     eqproto = save(e_proto);
> -     hproto = save(h_proto);
> -     wproto = save(w_proto);
> +     prproto[0] = estrdup(s_proto);
> +     prproto[1] = estrdup(less_is_more ? more_proto : m_proto);
> +     prproto[2] = estrdup(less_is_more ? more_M_proto : M_proto);
> +     eqproto = estrdup(e_proto);
> +     hproto = estrdup(h_proto);
> +     wproto = estrdup(w_proto);
>  }
>  
>  /*
> Index: tags.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/less/tags.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 tags.c
> --- tags.c    6 Nov 2015 15:50:33 -0000       1.13
> +++ tags.c    7 Nov 2015 04:04:50 -0000
> @@ -112,13 +112,13 @@ maketagent(char *file, LINENUM linenum, 
>       struct tag *tp;
>  
>       tp = ecalloc(sizeof (struct tag), 1);
> -     tp->tag_file = save(file);
> +     tp->tag_file = estrdup(file);
>       tp->tag_linenum = linenum;
>       tp->tag_endline = endline;
>       if (pattern == NULL)
>               tp->tag_pattern = NULL;
>       else
> -             tp->tag_pattern = save(pattern);
> +             tp->tag_pattern = estrdup(pattern);
>       return (tp);
>  }
>  
> 

Reply via email to