Hmm I thought this was deliberate but looking at where estrdup is used
it is obviously not. ok nicm
On Fri, Nov 06, 2015 at 11:07:26PM -0500, 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.
>
>
> 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);
> }
>
>
