On Sun, Sep 17, 2017 at 02:56:32AM +0000, Scott Cheloha wrote:
> Hi,
>
> This will make mg(1) ever so slightly more portable: downstream
> will appreciate it. And fgetln(3) was recently deprecated.
>
> Misc. comments:
>
> The comments about NUL and "the last line problem" aren't per se
> applicable with getline(3) so I removed them.
>
> I will say that if ever a command does *not* emit a trailing
> newline that we will stub out the last byte, but I decided to take
> the comments' authors at their word and continued to assume the
> presence of a newline. This can be rectified with:
>
> if (buf[len - 1] == '\n')
> buf[len - 1] = '\0';
the diff reads fine to me, but code tends to get copied around,
can you send a diff with those lines added please? That's the
canonical way of using getline I believe and there is no good
reason for mg to be different, thanks!
>
> The '};' in do_cscope() looked like a typo so I deleted the
> semicolon.
>
> I also noticed that there were no error checks after the read loops
> so I added an echo print on ferror(). I don't know if this is
> sufficient, but we weren't doing anything before, so it's a start.
>
> Thoughts?
>
> --
> Scott Cheloha
>
> Index: usr.bin/mg/cscope.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/cscope.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 cscope.c
> --- usr.bin/mg/cscope.c 19 Jan 2016 14:51:00 -0000 1.16
> +++ usr.bin/mg/cscope.c 16 Sep 2017 22:44:50 -0000
> @@ -166,9 +166,13 @@ cscreatelist(int f, int n)
> struct stat sb;
> FILE *fpipe;
> char dir[NFILEN], cmd[BUFSIZ], title[BUFSIZ], *line, *bufp;
> - size_t len;
> + size_t sz;
> + ssize_t len;
> int clen;
>
> + line = NULL;
> + sz = 0;
> +
> if (getbufcwd(dir, sizeof(dir)) == FALSE)
> dir[0] = '\0';
>
> @@ -221,11 +225,13 @@ cscreatelist(int f, int n)
> }
> addline(bp, title);
> addline(bp, "");
> - /* All lines are NUL terminated */
> - while ((line = fgetln(fpipe, &len)) != NULL) {
> + while ((len = getline(&line, &sz, fpipe)) != -1) {
> line[len - 1] = '\0';
> addline(bp, line);
> }
> + free(line);
> + if (ferror(fpipe))
> + ewprintf("Problem reading pipe");
> pclose(fpipe);
> return (popbuftop(bp, WNONE));
> }
> @@ -397,7 +403,11 @@ do_cscope(int i)
> char pattern[MAX_TOKEN], cmd[BUFSIZ], title[BUFSIZ];
> char *p, *buf;
> int clen, nores = 0;
> - size_t len;
> + size_t sz;
> + ssize_t len;
> +
> + buf = NULL;
> + sz = 0;
>
> /* If current buffer isn't a source file just return */
> if (fnmatch("*.[chy]", curbp->b_fname, 0) != 0) {
> @@ -447,13 +457,17 @@ do_cscope(int i)
> addline(bp, title);
> addline(bp, "");
> addline(bp,
> "-------------------------------------------------------------------------------");
> - /* All lines are NUL terminated */
> - while ((buf = fgetln(fpipe, &len)) != NULL) {
> + while ((len = getline(&buf, &sz, fpipe)) != -1) {
> buf[len - 1] = '\0';
> - if (addentry(bp, buf) != TRUE)
> + if (addentry(bp, buf) != TRUE) {
> + free(buf);
> return (FALSE);
> + }
> nores = 1;
> - };
> + }
> + free(buf);
> + if (ferror(fpipe))
> + ewprintf("Problem reading pipe");
> pclose(fpipe);
> addline(bp,
> "-------------------------------------------------------------------------------");
> if (nores == 0)
> Index: usr.bin/mg/grep.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/mg/grep.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 grep.c
> --- usr.bin/mg/grep.c 19 Mar 2015 21:48:05 -0000 1.44
> +++ usr.bin/mg/grep.c 16 Sep 2017 22:44:50 -0000
> @@ -178,12 +178,16 @@ compile_mode(const char *name, const cha
> struct buffer *bp;
> FILE *fpipe;
> char *buf;
> - size_t len;
> + size_t sz;
> + ssize_t len;
> int ret, n;
> char cwd[NFILEN], qcmd[NFILEN];
> char timestr[NTIME];
> time_t t;
>
> + buf = NULL;
> + sz = 0;
> +
> n = snprintf(qcmd, sizeof(qcmd), "%s 2>&1", command);
> if (n < 0 || n >= sizeof(qcmd))
> return (NULL);
> @@ -210,15 +214,13 @@ compile_mode(const char *name, const cha
> ewprintf("Problem opening pipe");
> return (NULL);
> }
> - /*
> - * We know that our commands are nice and the last line will end with
> - * a \n, so we don't need to try to deal with the last line problem
> - * in fgetln.
> - */
> - while ((buf = fgetln(fpipe, &len)) != NULL) {
> + while ((len = getline(&buf, &sz, fpipe)) != -1) {
> buf[len - 1] = '\0';
> addline(bp, buf);
> }
> + free(buf);
> + if (ferror(fpipe))
> + ewprintf("Problem reading pipe");
> ret = pclose(fpipe);
> t = time(NULL);
> strftime(timestr, sizeof(timestr), "%a %b %e %T %Y", localtime(&t));
--
I'm not entirely sure you are real.
