On Mon, Jun 18, 2018 at 06:43:14PM -0500, Taylor Blau wrote:

>  static void show_line(struct grep_opt *opt, char *bol, char *eol,
> -                   const char *name, unsigned lno, char sign)
> +                   const char *name, unsigned lno, unsigned cno, char sign)

Here "cno" is unsigned. But later...

> +     if (opt->columnnum && cno) {
> +             char buf[32];
> +             xsnprintf(buf, sizeof(buf), "%d", cno);

...we print it with "%d". Should this be "%u"?

But ultimately, the column number comes from this code:

> @@ -1785,6 +1796,7 @@ static int grep_source_1(struct grep_opt *opt, struct 
> grep_source *gs, int colle
>       while (left) {
>               char *eol, ch;
>               int hit;
> +             ssize_t cno;
>               ssize_t col = -1, icol = -1;
>  
>               /*
> @@ -1850,7 +1862,15 @@ static int grep_source_1(struct grep_opt *opt, struct 
> grep_source *gs, int colle
>                               show_pre_context(opt, gs, bol, eol, lno);
>                       else if (opt->funcname)
>                               show_funcname_line(opt, gs, bol, lno);
> -                     show_line(opt, bol, eol, gs->name, lno, ':');
> +                     cno = opt->invert ? icol : col;
> +                     if (cno < 0) {
> +                             /*
> +                              * A negative cno means that there was no match.
> +                              * Clamp to the beginning of the line.
> +                              */
> +                             cno = 0;
> +                     }

...which is a ssize_t. Should we just be using ssize_t consistently?

We do at least clamp the negative values here, but on 64-bit systems
ssize_t is much larger than "unsigned".  I admit that it's probably
ridiculous for any single line to overflow 32 bits, but it seems like we
should consistently use size_t/ssize_t for buffer offsets, and then we
don't have to think about it.

-Peff

Reply via email to