Re: Fwd: Re: fmtcheck() query

[Valery Ushakov <uwe%stderr.spb.ru@localhost>]

On Sat, Dec 09, 2017 at 09:43:56 +0900, Rin Okuyama wrote:

> We are discussing how fmtcheck(3) should be. Previously, it allows
> format strings consuming less arguments than the default one, as long
> as consumed args are of correct types. Recently, I've changed it to
> reject such a string as documented in man page. However, we are not
> sure which behavior is preferable.
> 
> Both may be incomplete. fmtcheck(3) should support positional args.
> Then, format strings should be allowed if consumed args have correct
> types, and the number of args is less than or equal to the default one.
> 
> How do you think? And how is your version?

Ok, it seems to be coming back slowly now...

It started when I was cleaning up usr.bin/man in July 2013.  In
man.conf(5) you can specify shell commands with _build and _crunch
keywords so I thought that it would be nice to verify them like gcc
does statically.  I didn't know that we had fmtcheck(3) already, so I
hacked together a quick prototype over the weekend, but didn't
complete it.

Positional arguments are not in ANSI C, I used SUS as my reference:
http://pubs.opengroup.org/onlinepubs/007908799/xsh/fprintf.html

This prototype was lying around until November when Christos found out
about it and provided some feedback.  So I hacked on it a bit more and
that's what's in that hg repo.

My version doesn't support %p and %n.  IIRC this should be trivial to
add.

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.

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 haven't thought about the effects of promotions vs ellipsis at all.

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.

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.


So after hacking on it a bit more that one weekend I dropped it again.
IIRC b/c it was the boring part of figuring out the good API for error
reporting etc.  E.g. I strongly dislike fmtcheck() return value, it
seduces you into using fmtcheck() call directly as the format string
argument.

Eventually I've committed a fix to man to use fmtcheck() along with
this:

  /*
   * A do-nothing counterpart to fmtcheck(3) that only supplies the
   * __format_arg marker.  Actual fmtcheck(3) call is done once in
   * config().
   */
  __always_inline __format_arg(2)
  static inline const char *
  fmtcheck_ok(const char *userfmt, const char *template)
  {
          return userfmt;
  }

used like:

  snprintf(cmd, sizeof(cmd), fmtcheck_ok(buf, "%s"), p);

Of course this depends on the actual call to fmtcheck() to be made
with the same template format string.

With those fixes committed we could drop -Wno-format-nonliteral.  And
then - out of sight, out of mind - I forgot about this code.

If you want to use it to check the nvi catalog, then I guess you
should be able to just use it right away, probably as-is.  The test
code contains main() that checks argv[2] format against argv[1]
template, e.g.

    $ ./printf_checkformat '%d' 'r=0x%1$08x (%1$d)'

Note that the order is the inverse compared to fmtcheck().

Hope that helps.  Sorry for the saga.

-uwe