Re: Fwd: Re: fmtcheck() query

[Rin Okuyama <rokuyama%rk.phys.keio.ac.jp@localhost>]

Thank you very much for the thorough explanation, and I'm sorry for the
late reply.

On 2017/12/10 2:04, Valery Ushakov wrote:
> My version doesn't support %p and %n.  IIRC this should be trivial to
> add.

It was actually trivial. I've added them.

> It permits signed/unsigned confusion (e.g. it will accept format %u as
> ok given the %d template) - iirc, it's needed to let e.g. %x stand for
> %d.

This is the same behavior with fmtcheck(3).

> I didn't bother with "lose" matches that take actual type widths into
> account (e.g. accept %ld for %d if type sizes happen to be the same on
> the host) - I don't think this should be allowed even as an option.

I agree with you.

> I haven't thought about the effects of promotions vs ellipsis at all.

fmtcheck(3) also doesn't take care of them, but I'm not sure whether
this causes real problems or not...

> My version currently complains about unused arguments, but there's an
> XXX comment about that.  For the numbered case the relevant passage
> is:
>
>    "When numbered argument specifications are used, specifying the Nth
>    argument requires that all the leading arguments, from the first to
>    the (N-1)th, are specified in the format string."
>
> i.e. it's ok to consume less, but you have to consume all the ones
> before.  The latter is obviously not a concern for the usual
> sequential access.

Due to this statement of SUS (it is found also in SUSv4
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html),
it is quite inconvenient to neglect some arguments *intentionally* in
the framework of printf(3); Missing some arguments in format strings
should probably be a mistake. I therefore agree with the current
behavior.

> I don't remember why the code complains if the tempalte refers to the
> same argument multiple times.  I guess I was too lazy to make sure the
> two references are compatible.

It should be allowed, as long as types are consistent, since SUSv4 says:

    "In format strings containing the "%n$" form of conversion
    specification, numbered arguments in the argument list can be
    referenced from the format string as many times as required."

I've changed it to complain only if the same argument is referenced as
incompatible types.

Let us back to the original topic, how fmtcheck(3) be. I think it should
be replaced by that based on printf_checkformat(). Then,

- Positional arguments are supported.
- Unused arguments are treated as an error; see reasoning above.

Thoughts?

rin
----
--- printf_checkformat.c.orig	2017-12-11 15:58:05.000000000 +0900
+++ printf_checkformat.c	2017-12-11 22:04:56.000000000 +0900
@@ -39,6 +39,7 @@
 #include <stdio.h>
 #include <string.h>
 #ifdef TEST
+#include <err.h>
 #include <getopt.h>
 #endif
 
@@ -76,6 +77,17 @@
 	A_STRING,		/* %s */
 	A_WIDE_STRING,		/* %ls */
 
+	A_POINTER,		/* %p */
+
+	A_INT_POINTER,		/* %n */
+	A_CHAR_POINTER,		/* ... with hh modifier */
+	A_SHORT_POINTER,	/* ... with h modifier */
+	A_LONG_POINTER,		/* ... with l modifier */
+	A_LONG_LONG_POINTER,	/* ... with ll modifier */
+	A_SIZE_POINTER,		/* ... with z modifier */
+	A_INTMAX_POINTER,	/* ... with j modifier */
+	A_PTRDIFF_POINTER,	/* ... with t modifier */
+
 	A_ASTERISK,		/* variable width/precision */
 };
 
@@ -419,10 +431,42 @@
 			break;
 
 		case 'p':
-			FMTERR("%%p is not allowed");
+			if (lenmod != LM_NONE)
+				FMTERR("%%p doesn't take modifiers");
+			CALLBACK(arg, A_POINTER);
+			break;
 
 		case 'n':
-			FMTERR("%%n is not allowed");
+			switch (lenmod) {
+			case LM_NONE:
+				CALLBACK(arg, A_INT_POINTER);
+				break;
+			case LM_hh:
+				CALLBACK(arg, A_CHAR_POINTER);
+				break;
+			case LM_h:
+				CALLBACK(arg, A_SHORT_POINTER);
+				break;
+			case LM_l:
+				CALLBACK(arg, A_LONG_POINTER);
+				break;
+			case LM_ll:
+				CALLBACK(arg, A_LONG_LONG_POINTER);
+				break;
+			case LM_j:
+				CALLBACK(arg, A_INTMAX_POINTER);
+				break;
+			case LM_z:
+				CALLBACK(arg, A_SIZE_POINTER);
+				break;
+			case LM_t:
+				CALLBACK(arg, A_PTRDIFF_POINTER);
+				break;
+			case LM_L:
+				FMTERR("%%n doesn't take 'L' modifier");
+				break;
+			}
+			break;
 
 		default:
 			FMTERR("no conversion specifier");
@@ -453,6 +497,17 @@
 	[A_STRING] = (1U << A_STRING),
 	[A_WIDE_STRING] = (1U << A_WIDE_STRING),
 
+	[A_POINTER] = (1U << A_POINTER),
+
+	[A_INT_POINTER] = (1U << A_INT_POINTER),
+	[A_CHAR_POINTER] = (1U << A_CHAR_POINTER),
+	[A_SHORT_POINTER] = (1U << A_SHORT_POINTER),
+	[A_LONG_POINTER] = (1U << A_LONG_POINTER),
+	[A_LONG_LONG_POINTER] = (1U << A_LONG_LONG_POINTER),
+	[A_SIZE_POINTER] = (1U << A_SIZE_POINTER),
+	[A_INTMAX_POINTER] = (1U << A_INTMAX_POINTER),
+	[A_PTRDIFF_POINTER] = (1U << A_PTRDIFF_POINTER),
+
 	[A_ASTERISK] = (1U << A_ASTERISK),
 };
 
@@ -496,9 +551,10 @@
 
 	aidx = arg - 1;
 	if (fm->template[aidx] != A_UNDEF
-	    && !(fm->template[aidx] == A_ASTERISK && spec == A_ASTERISK))
+	    && (compatible_specs[fm->template[aidx]] & ~compatible_specs[spec]))
 	{
-		errmsg(fm, "template refers to argument %zu multiple times", arg);
+		errmsg(fm, "template refers to argument %zu multiple times"
+		    " as inconsistent types", arg);
 		return EINVAL;
 	}
 
@@ -663,6 +719,17 @@
 	[A_STRING] = "STRING",
 	[A_WIDE_STRING] = "WIDE STRING",
 
+	[A_POINTER] = "POINTER",
+
+	[A_INT_POINTER] = "INT_POINTER",
+	[A_CHAR_POINTER] = "CHAR_POINTER",
+	[A_SHORT_POINTER] = "SHORT_POINTER",
+	[A_LONG_POINTER] = "LONG_POINTER",
+	[A_LONG_LONG_POINTER] = "LONG_LONG_POINTER",
+	[A_SIZE_POINTER] = "SIZE_POINTER",
+	[A_INTMAX_POINTER] = "INTMAX_POINTER",
+	[A_PTRDIFF_POINTER] = "PTRDIFF_POINTER",
+
 	[A_ASTERISK] = "ASTERISK",
 };