> -     free(allocated);
> +     if (allocated)
> +             free(allocated);

This is unnecessary, since free(NULL) is clearly defined as a no-op.
See the malloc(3) man page.

Tom

>>> Nan Xiao 4-Sep-17 12:11 >>>
>
> Hi tech@,
>
> This patch fixes the extreme case in dmesg.c: if memf or nlistf is not
> NULL, and "NOKVM" macro is defined.
>
> Current code in dmesg.c:
>
>       struct msgbuf cur;
>       
> Since "cur" is not initialized, so the following code has undefined
> behavior:
>
>       if (cur.msg_bufx >= cur.msg_bufs)
>               cur.msg_bufx = 0;
>       /*
>        * The message buffer is circular; start at the read pointer, and
>        * go to the write pointer - 1.
>        */
>       for (newl = skip = i = 0, p = bufdata + cur.msg_bufx;
>           i < cur.msg_bufs; i++, p++) {
>       .....
>       }
>
> My patch can skip the whole loop, and the "dmesg" program just prints
> a newline:
>
>       if (!newl)
>               putchar('\n');
>
> Best Regards
> Nan Xiao
>
> Index: dmesg.c
> ===================================================================
> RCS file: /cvs/src/sbin/dmesg/dmesg.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 dmesg.c
> --- dmesg.c   1 Sep 2017 07:31:45 -0000       1.29
> +++ dmesg.c   4 Sep 2017 08:55:50 -0000
> @@ -65,12 +65,12 @@ main(int argc, char *argv[])
>       int ch, newl, skip, i;
>       char *p;
>       struct msgbuf cur;
> -     char *memf, *nlistf, *bufdata = NULL;
> +     char *memf = NULL, *nlistf = NULL, *bufdata = NULL;
>       char *allocated = NULL;
>       int startupmsgs = 0;
>       char buf[5];
>
> -     memf = nlistf = NULL;
> +     memset(&cur, 0, sizeof(cur));
>       while ((ch = getopt(argc, argv, "sM:N:")) != -1)
>               switch(ch) {
>               case 's':
> @@ -184,7 +184,8 @@ main(int argc, char *argv[])
>       }
>       if (!newl)
>               putchar('\n');
> -     free(allocated);
> +     if (allocated)
> +             free(allocated);
>       return (0);
>  }

Reply via email to