awk: avoid walking off end of some invalid format strings

[emaste]

Summary

This is a minimal change to avoid the invalid memory access; a more
comprehensive change ought to happen upstream.

Reported by:    phk
Sponsored by:   The FreeBSD Foundation

[imp]

I don't understand why you did what you did worked... Perhaps a sentence or two explaining why that's the right place to do this.

Also, what does the test case printf("%#") or %- or other pathology do? Or some other unterminated sequence?

contrib/one-true-awk/run.c

851

Why not have that

if (*(s+1) == '\0') break;

here, either before or after this if.

[emaste]

contrib/one-true-awk/run.c

851

That would exit the loop before detecting and reporting the error if the string is just "%", and would not fix the error phk reported for e.g. %1

[emaste]

I don't understand why you did what you did worked... Perhaps a sentence or two explaining why that's the right place to do this.

This was the simplest way to avoid walking off the end of the string, if we've already consumed everything. I don't think it's the "right" place to do this, it's just the least invasive.

[emaste]

Add a comment about the break condition.

[emaste]

contrib/one-true-awk/run.c

861

This s++ is the offending increment, moving s to the \0. But we want to keep the warnings at line 916/921, so we don't want to exit the loop here.

[emaste]

small improvement: move the \0 check into the offending loop

[markj]

contrib/one-true-awk/run.c

861

Should the order of these checks be reversed? That is, don't you want to know that *s != '\0' before checking whether *(s+1) != '\0'?

[emaste]

sample output w/ this change:

$ ./a.out 'BEGIN {printf("%3u\n", 123)}'
123

$ ./a.out 'BEGIN {printf("%3u\n")}'
./a.out: not enough args in printf(%3u
)
 source line number 1

 $ ./a.out 'BEGIN {printf("%3\n", 123)}'
./a.out: weird printf conversion %3
 source line number 1
%3123

$ ./a.out 'BEGIN {printf("%")}'
./a.out: weird printf conversion 
 source line number 1
./a.out: not enough args in printf(%)
 source line number 1

[emaste]

contrib/one-true-awk/run.c

861

Hmm, yes - I did it in this order because I think we don't want the t++ for the case that we lack the conversion character.

In practice I think it's actually OK (although unintentional) but is indeed confusing. What about

for (t = fmt; *s != '\0' && *(s+1) != '\0'; s++) {
        *t++ = *s;

(I say it's OK because on the first loop iteration *s is not \0 (from line 845), and so it's fine to examine *(s+1) on each pass.)

[imp]

This looks good to me. I'm neutral on order per mark.

[emaste]

third try: just turn missing conversion specifier into a fatal error

[emaste]

contrib/one-true-awk/run.c

916

will add break; as well to keep Coverity or other tools happy

[imp]

thanks for making this much better.

[markj]

This seems sane.