Thread: BUG #15511: Drop table error "invalid argument"

The following bug has been logged on the website:

Bug reference:      15511
Logged by:          Ertuğrul Kahveci
Email address:      ertugrul9090@gmail.com
PostgreSQL version: 11.1
Operating system:   windows 10
Description:

Hello;
         Drop table error "HATA: vsnprintf failed: Invalid argument", error
code:"XX000" , PostgreSQL version..: "11.1"
 Thanks,

Hi,

On 2018-11-17 20:32:27 +0000, PG Bug reporting form wrote:
> The following bug has been logged on the website:
> 
> Bug reference:      15511
> Logged by:          Ertuğrul Kahveci
> Email address:      ertugrul9090@gmail.com
> PostgreSQL version: 11.1
> Operating system:   windows 10
> Description:        
> 
> Hello;
>          Drop table error "HATA: vsnprintf failed: Invalid argument", error
> code:"XX000" , PostgreSQL version..: "11.1"
>  Thanks,

This is not enough information for us to do anything abou this
report. What precisely lead to this error being reported? Can you give
us enough details to reproduce?

Do you get the same error if you do SET lc_messages = 'C'; before you
drop the table?

Greetings,

Andres Freund

=?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
>          Drop table error "HATA: vsnprintf failed: Invalid argument", error
> code:"XX000" , PostgreSQL version..: "11.1"

Hmm.  Looking at snprintf.c, EINVAL could only be returned for an
incorrect translated message (i.e. wrong use of %n$ notation),
so it seems somebody fat-fingered a translation.  We can infer
that you're using the Turkish message set, but that doesn't help
much to narrow down where the mistake is.  What do you see if you
do "set lc_messages = 'C'" and then repeat the failing command?

            regards, tom lane

I wrote:
> =?utf-8?q?PG_Bug_reporting_form?= <noreply@postgresql.org> writes:
>> Drop table error "HATA: vsnprintf failed: Invalid argument", error
>> code:"XX000" , PostgreSQL version..: "11.1"

> Hmm.  Looking at snprintf.c, EINVAL could only be returned for an
> incorrect translated message (i.e. wrong use of %n$ notation),
> so it seems somebody fat-fingered a translation.  We can infer
> that you're using the Turkish message set, but that doesn't help
> much to narrow down where the mistake is.  What do you see if you
> do "set lc_messages = 'C'" and then repeat the failing command?

I poked at that to the extent of testing every string in 11.1's tr.po
against our implementation of snprintf, and what I find is this:

#. translator: second %s is, e.g., "table %s"
#: catalog/objectaddress.c:2694
#, c-format
msgid "column %s of %s"
msgstr "%2$s'nin %1$ sütunu"

This msgstr is legal according to POSIX, so it's not surprising that
GNU msgfmt doesn't complain about it; but our version of snprintf()
does, because space is not a valid flag character according to it.
So the bit "%1$ s" is valid to msgfmt but not to us.

Presumably, what failed for you was something along the lines of

regression=# create table foo (f1 int);
CREATE TABLE
regression=# create table bar (f1 foo);
CREATE TABLE
regression=# drop table foo;
ERROR:  cannot drop table foo because other objects depend on it
DETAIL:  column f1 of table bar depends on type foo
HINT:  Use DROP ... CASCADE to drop the dependent objects too.

Conclusions:

(1) I don't know Turkish, but it seems clear that this is a typo
and the string ought to be

msgstr "%2$s'nin %1$s sütunu"

(2) It seems like a bad idea that pvsnprintf(), which must have
reported this message, knows full well that it's dealing with a
broken format string and yet doesn't print out that format string.
It would have taken much less work to find this problem if it had.

(3) I'm quite unwilling to try to make snprintf.c accept absolutely
everything that's in the POSIX spec, especially seeing that this
particular omission caught a mistake.

(4) However, that leaves us with a translation problem, because msgfmt
doesn't detect some things that we'll fail on at runtime.  What shall
we do about that?

            regards, tom lane

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> (4) However, that leaves us with a translation problem, because msgfmt
> doesn't detect some things that we'll fail on at runtime.  What shall
> we do about that?

I realize this is pretty pie-in-the-sky, but maybe we should have a
buildfarm member that's actually running the regression suite with every
translation we support..?  Of course, then we should also have our
regression tests actually causing every error message to be hit at least
once or we'd still possibly miss things.

A shorter path, though one with less additional benefits, would
presumably be to find a way to make msgfmt fail on things that we'll
fail on..?  Or otherwise detect such cases..?

Thanks!

Stephen

Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> (4) However, that leaves us with a translation problem, because msgfmt
>> doesn't detect some things that we'll fail on at runtime.  What shall
>> we do about that?

> A shorter path, though one with less additional benefits, would
> presumably be to find a way to make msgfmt fail on things that we'll
> fail on..?  Or otherwise detect such cases..?

What I did to locate the broken translation in the first place was
to make a heavily lobotomized version of snprintf.c which would detect
EINVAL problems in a supplied format string, without trying to actually
fetch any values or emit any output.  Then I applied it to every string
in tr.po.  This was a complete kluge only suitable for one-shot use,
but you could imagine making something a bit more production-ready
and then inserting it into the translation pipeline somewhere.  It'd
likely be worth applying it to the original strings as well as the
translated ones, in case we have any seldom-executed code in which
the original string is bad itself.

I noted while looking at the problem that it's somewhat accidental
that snprintf.c reports EINVAL rather than just producing bogus
output for this case.  If we were planning to go this direction,
I'd be inclined to make it report EINVAL for any situation where
it hits an unrecognized format code --- ie put an error into the
default cases in the switches there.

That's just handwaving though, as I have little idea what the
"translation pipeline" looks like.

            regards, tom lane

On 2018-Nov-17, Tom Lane wrote:

> I poked at that to the extent of testing every string in 11.1's tr.po
> against our implementation of snprintf, and what I find is this:
> 
> #. translator: second %s is, e.g., "table %s"
> #: catalog/objectaddress.c:2694
> #, c-format
> msgid "column %s of %s"
> msgstr "%2$s'nin %1$ sütunu"

I fixed this string in tr.po.  At least it'll be correct in 11.2.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2018-Nov-17, Tom Lane wrote:
>> I poked at that to the extent of testing every string in 11.1's tr.po
>> against our implementation of snprintf, and what I find is this:
>> msgid "column %s of %s"
>> msgstr "%2$s'nin %1$ sütunu"

> I fixed this string in tr.po.  At least it'll be correct in 11.2.

Thanks!  I'm also just about to push code changes for the easy parts
of what I recommended earlier (have psprintf report the bogus format
string, and make snprintf.c throw EINVAL for more error cases).

Is anybody interested in the idea of making our own validator for
the .po files?  I might be willing to code it up, if I knew what
its API ought to be, but I don't know the workflow in that area.

            regards, tom lane

On 2018-Dec-06, Tom Lane wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > On 2018-Nov-17, Tom Lane wrote:
> >> I poked at that to the extent of testing every string in 11.1's tr.po
> >> against our implementation of snprintf, and what I find is this:
> >> msgid "column %s of %s"
> >> msgstr "%2$s'nin %1$ sütunu"
> 
> > I fixed this string in tr.po.  At least it'll be correct in 11.2.
> 
> Thanks!  I'm also just about to push code changes for the easy parts
> of what I recommended earlier (have psprintf report the bogus format
> string, and make snprintf.c throw EINVAL for more error cases).

Great.

> Is anybody interested in the idea of making our own validator for
> the .po files?  I might be willing to code it up, if I knew what
> its API ought to be, but I don't know the workflow in that area.

I think it's definitely worthwhile to avoid introducing bogus files in
the Pg repo.

The work is done by these scripts:
https://git.postgresql.org/gitweb/?p=pgtranslation/admin.git;a=tree;h=refs/heads/master;hb=refs/heads/master
cp-po does some validation while copying the file from the pgtranslation
repo back to Postgres, but I think that's the wrong time (at release
time).  The good one I think is wwwtools/pg-make-po, which runs a few
times daily and produces the error file that ends up in the status table
in babel.postgresql.org.  We'd want something that produces output that
can be appended to the $outdir/$catalogname-$lang.po.err file (lines
103ff of that script).

Peter is the authority on this, of course.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2018-Dec-06, Tom Lane wrote:
>> Is anybody interested in the idea of making our own validator for
>> the .po files?  I might be willing to code it up, if I knew what
>> its API ought to be, but I don't know the workflow in that area.

> I think it's definitely worthwhile to avoid introducing bogus files in
> the Pg repo.

> The work is done by these scripts:
> https://git.postgresql.org/gitweb/?p=pgtranslation/admin.git;a=tree;h=refs/heads/master;hb=refs/heads/master
> cp-po does some validation while copying the file from the pgtranslation
> repo back to Postgres, but I think that's the wrong time (at release
> time).  The good one I think is wwwtools/pg-make-po, which runs a few
> times daily and produces the error file that ends up in the status table
> in babel.postgresql.org.  We'd want something that produces output that
> can be appended to the $outdir/$catalogname-$lang.po.err file (lines
> 103ff of that script).

Hm.  So that's all Perl code...

In principle, we could write some Perl code that exactly matches what
snprintf.c thinks is valid input, but I think that keeping it in sync
would be a nightmare.  The concept I had in mind was to make a variant
version of snprintf.c that just validates a format string, and can be
compared to snprintf.c by diff'ing.  (Or, perhaps, sprinkle snprintf.c
with #ifdefs so that compiling it with the right -D flag produces what
we want; though that might look too ugly.)  If you don't mind adding
a C compiler to the list of dependencies for pg-make-po, we could imagine
having it compile up such a program at startup and then apply it to
each catalog.

> Peter is the authority on this, of course.

Yup.  Peter, any comments?

            regards, tom lane

On 2018-Dec-06, Tom Lane wrote:

> In principle, we could write some Perl code that exactly matches what
> snprintf.c thinks is valid input, but I think that keeping it in sync
> would be a nightmare.  The concept I had in mind was to make a variant
> version of snprintf.c that just validates a format string, and can be
> compared to snprintf.c by diff'ing.  (Or, perhaps, sprinkle snprintf.c
> with #ifdefs so that compiling it with the right -D flag produces what
> we want; though that might look too ugly.)  If you don't mind adding
> a C compiler to the list of dependencies for pg-make-po, we could imagine
> having it compile up such a program at startup and then apply it to
> each catalog.

I don't follow.  Why don't we just compile snprintf.c as-is and another
.c file with a function that invokes vsnprintf on each translated string
on a .po file and prints an error if vsnprintf returns EINVAL?

This code runs completely under our control, and we can install whatever
tools are needed.  We don't need a C compiler today, but installing one
is trivial.  Also, we already have postgres source trees for each PG
version available.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2018-Dec-06, Tom Lane wrote:
>> In principle, we could write some Perl code that exactly matches what
>> snprintf.c thinks is valid input, but I think that keeping it in sync
>> would be a nightmare.  The concept I had in mind was to make a variant
>> version of snprintf.c that just validates a format string, and can be
>> compared to snprintf.c by diff'ing.  (Or, perhaps, sprinkle snprintf.c
>> with #ifdefs so that compiling it with the right -D flag produces what
>> we want; though that might look too ugly.)  If you don't mind adding
>> a C compiler to the list of dependencies for pg-make-po, we could imagine
>> having it compile up such a program at startup and then apply it to
>> each catalog.

> I don't follow.  Why don't we just compile snprintf.c as-is and another
> .c file with a function that invokes vsnprintf on each translated string
> on a .po file and prints an error if vsnprintf returns EINVAL?

Because figuring out what additional arguments go with a particular
format string is pretty hard, and we can't just not do that, or
snprintf will likely dump core.  We need a lobotomized version that
doesn't attempt to fetch any of the variable parameters.

> This code runs completely under our control, and we can install whatever
> tools are needed.  We don't need a C compiler today, but installing one
> is trivial.  Also, we already have postgres source trees for each PG
> version available.

OK.  I'd visualize only adding the test code to HEAD, and pulling it from
there to use for all back branches.

            regards, tom lane

... so my preliminary result is that there's a *boatload* of "invalid"
format strings in our translation files, all of them apparently with
the extra-space disease.  See attached.

While it looks like every one of these is a typo, I'm beginning to
have second thoughts about the premise here.  Presumably, the reason
we've not noticed this issue before is that almost all non-Windows
platforms were using libc's printf, which is going to implement the
full set of POSIX-specified flags including space.  As of HEAD that
is no longer the case, because we use snprintf.c everywhere.  While
it might not be a problem for our own code if snprintf.c is a couple
of flags short of full POSIX, I'm worrying that third-party code
might be unhappy if *printf in the PG environment behaves oddly.

So maybe the best compromise is to upgrade snprintf.c.  I think it'd
only take a few more lines to implement the space flag per spec.
The other flags that we aren't implementing are '#' ("alternate form")
and "'" (thousands grouping).  For those, I'd be a bit inclined to accept
them but treat 'em as no-ops.  On the other hand, that still leaves
some daylight between us and POSIX, in the form of stuff like "long
double" support, and I still don't want to go near that.

Thoughts?

For the archives' sake, I attach the code I used for this.
I'm now thinking maybe we wouldn't commit it, though.

            regards, tom lane

invalid format string in file "src/bin/initdb/po/he.po" for ../../common/restricted_token.c:77
"תכנית %s: לא ניתן לפתוח את התהליך token: קוד שגיאה % lu
"
invalid format string in file "src/bin/initdb/po/he.po" for ../../common/restricted_token.c:90
"תכנית %s: לא ניתן להקצות SID: קוד שגיאה % lu
"
invalid format string in file "src/bin/initdb/po/he.po" for ../../common/restricted_token.c:132
"תכנית %s:  לא ניתן להפעיל תהליך עבור הפקודה "%s": קוד שגיאה % lu
"
invalid format string in file "src/bin/initdb/po/he.po" for ../../common/restricted_token.c:170
"תכנית %s:  לא ניתן לבצע מחדש עם אסימון גישה מוגבל: קוד שגיאה % lu
"
invalid format string in file "src/bin/initdb/po/he.po" for ../../common/restricted_token.c:186
"תכנית %s:  לא ניתן לקבל קוד היציאה מן תהליך משנה: קוד שגיאה % lu
"
invalid format string in file "src/bin/initdb/po/he.po" for ../../common/username.c:43
"לא יכול לחפש יעיל את המשתמש עם מזהה % ld: %s"
invalid format string in file "src/bin/initdb/po/he.po" for ../../common/username.c:60
"כישלון בדיקה עבור שם המשתמש: קוד שגיאה % lu"
invalid format string in file "src/bin/initdb/po/tr.po" for initdb.c:2171
"% s: geçersiz yerel ayarlar; LANG ve LC_ * ortam değişkenlerini denetleyin.
"
invalid format string in file "src/bin/initdb/po/tr.po" for initdb.c:3195
"% s: superuser adı "% s" izin verilmiyor; rol adları "pg_" ile başlayamaz
"
invalid format string in file "src/bin/scripts/po/he.po" for ../../common/username.c:43
"לא יכול לחפש יעיל את המשתמש עם מזהה % ld: %s"
invalid format string in file "src/bin/scripts/po/he.po" for ../../common/username.c:60
"כישלון בדיקה עבור שם המשתמש: קוד שגיאה % lu"
invalid format string in file "src/bin/scripts/po/he.po" for ../../fe_utils/print.c:353
"(% lu שורה)"
invalid format string in file "src/bin/scripts/po/he.po" for ../../fe_utils/print.c:353
"(% lu שורה)"
invalid format string in file "src/bin/scripts/po/he.po" for ../../fe_utils/print.c:353
"(% lu שורות)"
invalid format string in file "src/bin/psql/po/he.po" for ../../common/username.c:43
"לא יכול לחפש יעיל את המשתמש עם מזהה % ld: %s"
invalid format string in file "src/bin/psql/po/he.po" for ../../common/username.c:60
"כישלון בדיקה עבור שם המשתמש: קוד שגיאה % lu"
invalid format string in file "src/bin/psql/po/he.po" for ../../fe_utils/print.c:353
"(% lu שורה)"
invalid format string in file "src/bin/psql/po/he.po" for ../../fe_utils/print.c:353
"(% lu שורה)"
invalid format string in file "src/bin/psql/po/he.po" for ../../fe_utils/print.c:353
"(% lu שורות)"
invalid format string in file "src/bin/psql/po/he.po" for command.c:553
"לא ניתן לקבל את ספריית הבית עבור משתמש עם מזהה % ld: %s
"
invalid format string in file "src/bin/psql/po/he.po" for common.c:608
"זמן: %.3f ms (% 02d:%06.3f)
"
invalid format string in file "src/bin/psql/po/he.po" for common.c:617
"זמן: %.3f ms (% 02d:%02d:%06.3f)
"
invalid format string in file "src/bin/pg_basebackup/po/he.po" for pg_basebackup.c:965
"תכנתי %s: לא היתה אפשרות להגדיר את רמת הדחיסה % d: %s
"
invalid format string in file "src/bin/pg_basebackup/po/he.po" for pg_recvlogical.c:135
"תכנית %s: מאשר לכתוב עד % X/%X, סומק ל %X/%X (חריץ %s)
"
invalid format string in file "src/bin/pg_basebackup/po/he.po" for receivelog.c:278
"תכנית %s: שרת דיווח שם קובץ היסטוריה לא צפוי עבור ציר הזמן % u: %s
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:495
"תכנית %s:  לא היתה אפשרות להפעיל שרת: קוד שגיאה % lu
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:967
"תכנית %s: לא יכול להפסיק את השרת; מופעל שרת למשתמש יחיד (PID: % ld)
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:975
"תכנית %s: לא היתה אפשרות לשלוח אות עצירה (PID: % ld): %s
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1058
"תכנית %s: לא ניתן להפעיל מחדש את השרת; מופעל שרת למשתמש יחיד (PID: % ld)
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1120
"תכנית %s: תהליך השרת הישן (PID: % ld) כנראה פועל
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1143
"תכנית %s: לא ניתן להפעיל מחדש את השרת; מופעל שרת למשתמש יחיד (PID: % ld)
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1152
"תכנית %s: לא היתה אפשרות לשלוח אות להפעלה מחדש (PID: % ld): %s
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1182
"תכנית %s: לא ניתן לקדם שרת; מופעל שרת למשתמש יחיד (PID: % ld)
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1219
"תכנית %s: לא היתה אפשרות לשלוח את האות  לקדם(PID: % ld): %s
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1301
"תכנית %s: שרת משתמש יחיד  פועל (PID: % ld)
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1314
"תכנית %s: שרת פועל (PID: % ld)
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1348
"תכנית %s: לא היתה אפשרות לשלוח אות %d (PID: % ld): %s
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1502
"תכנית %s: אין אפשרות לרשום את השירות "%s": קוד שגיאה % lu
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1532
"תכנית %s: לא היתה אפשרות לפתוח שירות '%s': קוד שגיאה % lu
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1541
"תכנית %s: לא יכול לבטל את השירות "%s": קוד שגיאה % lu
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1690
"תכנית %s: לא היתה אפשרות להפעיל שירות '%s': קוד שגיאה % lu
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1777
"תכנית %s: לא ניתן לפתוח את התהליך token: קוד שגיאה % lu
"
invalid format string in file "src/bin/pg_ctl/po/he.po" for pg_ctl.c:1791
"תכנית %s: לא ניתן להקצות SID: קוד שגיאה % lu
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:1302
"לא היתה אפשרות ליצור אובייקט גדול % u: %s"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:1307
"לא היתה אפשרות לפתוח את אובייקט גדול % u: %s"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:1652
"נכתב % lu בית של נתוני האובייקט הגדול (תוצאה = % lu)
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:1652
"נכתב % lu בית של נתוני האובייקט הגדול (תוצאה = % lu)
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:1652
"נכתבו % lu בתים של נתוני האובייקט הגדול (תוצאה = % lu)
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:1658
"לא מצליח לכתוב על אובייקט גדול (תוצאה: % lu, צפוי: % lu)
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:2147
"קובץ הקלט קצר מדי (לקרוא % lu, מצופה 5)
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:2562
"מזהה כניסה% d מחוץ לטווח - אולי TOC מושחת
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:3659
"גירסה לא נתמכת (% d.%d) בכותרת הקובץ
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_archiver.c:3664
"בדיקת שפיות על גודל מספר שלם (% lu) נכשלה
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_custom.c:451
"סוג בלוק נתונים לא מזוהה (% d) בעת חיפוש בארכיון
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_custom.c:472
"לא יכול למצוא את מזהה הבלוק % d בארכיון - אולי בשל בקשת שחזור שאינה מסודרת, שלא ניתן לטפל בה עקב חוסר קיזוז נתונים
בארכיון
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_custom.c:477
"לא יכול למצוא את מזהה הבלוק% d בארכיון - אולי בשל בקשת שחזור שאינה מסודרת, שלא ניתן לטפל בה עקב קובץ קלט שאינו ניתן
לחיפושבו 
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_custom.c:482
"לא יכול למצוא את מזהה הבלוק% d בארכיון - אולי בשל בקשת שחזור שאינה מסודרת, שלא ניתן לטפל בה עקב קובץ ארכיון מושחת
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_tar.c:1237
"כותרת tar לא שלמה נמצאה (% lu בית)
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_tar.c:1237
"כותרת tar לא שלמה נמצאה (% lu בית)
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_backup_tar.c:1237
"כותרת tar לא שלמה נמצאה (% lu בתים)
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_dump.c:1000
"צוין קידוד לקוח לא חוקי "% s"
"
invalid format string in file "src/bin/pg_dump/po/he.po" for pg_dump.c:3095
"שגיאה בעת קריאת אובייקט גדול % u: %s"
invalid format string in file "src/backend/po/id.po" for commands/vacuumlazy.c:1093
"% .0f mati versi baris tidak dapat dihapus belum.
Ada % .0f item pointer yang tidak terpakai.
halaman%u sepenuhnya kosong.
%s."
invalid format string in file "src/backend/po/id.po" for executor/functions.c:225
"tidak bisa menentukan pernyataan tipe argumen yang sebenarnya % s"
invalid format string in file "src/backend/po/id.po" for executor/functions.c:638
"tidak bisa menentukan hasil yang sebenarnya untuk pernyataan kepada pengembalian tipe % s"
invalid format string in file "src/backend/po/id.po" for libpq/auth.c:2203
"tidak dapat melakukan pengumpulan inisialisasi LDAP untuk ldapbinddn « %s » pada server « %s »:% s"
invalid format string in file "src/backend/po/id.po" for libpq/auth.c:2283
"tidak dapat melepaskan setelah mencari pengguna « %s » di server « %s »:% s"
invalid format string in file "src/backend/po/fr.po" for catalog/objectaddress.c:1903
"le droit par défaut pour l'utilisateur « % s» dans le schéma « %s » de %s n'existe pas"
invalid format string in file "src/interfaces/libpq/po/he.po" for fe-auth.c:1045
"כישלון בדיקה עבור שם המשתמש: קוד שגיאה % lu
"
invalid format string in file "src/interfaces/libpq/po/he.po" for fe-auth.c:1055
"לא יכול לחפש משתמש מקומי עם מזהה % d: %s
"
invalid format string in file "src/interfaces/libpq/po/he.po" for fe-connect.c:1561
"נכשל WSAIoctl(SIO_KEEPALIVE_VALS): % ui
"
invalid format string in file "src/interfaces/libpq/po/he.po" for fe-misc.c:292
"מספר שלם בגודל % lu אינו נתמך על ידי pqGetInt"
invalid format string in file "src/interfaces/libpq/po/he.po" for fe-misc.c:328
"מספר שלם בגודל % lu אינו נתמך על ידי pqPutInt"
invalid format string in file "src/interfaces/libpq/po/he.po" for fe-protocol2.c:529
"תו לא צפוי% c בעקבות תגובת שאילתה ריקה (הודעת "I")"
invalid format string in file "src/interfaces/libpq/po/he.po" for fe-protocol3.c:1316
"השורה % d: "
invalid format string in file "src/interfaces/libpq/po/he.po" for fe-secure-openssl.c:1465
"קוד שגיאת SSL % lu"
invalid format string in file "src/pl/plpython/po/vi.po" for plpy_typeio.c:1541
"thuộc tính "% s" không tồn tại trong đối tượng Python"
diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index a773381..42aaea8 100644
*** a/src/port/snprintf.c
--- b/src/port/snprintf.c
***************
*** 110,115 ****
--- 110,129 ----
  #undef    printf

  /*
+  * If this file is compiled with SNPRINTF_VALIDATE defined, it doesn't
+  * provide any of the normal *printf functions, but instead exports
+  * a function "pg_validate_printf_format()" that just detects whether a
+  * supplied format string is syntactically valid.  We implement that
+  * by running the normal snprintf code with a fake output buffer, and
+  * suppressing all attempts to fetch variable arguments (since those
+  * won't be there).
+  */
+ #ifdef SNPRINTF_VALIDATE
+ #undef va_arg
+ #define va_arg(args, type)  ((type) 0)
+ #endif
+
+ /*
   * Info about where the formatted output is going.
   *
   * dopr and subroutines will not write at/past bufend, but snprintf
*************** static void dopr(PrintfTarget *target, c
*** 170,175 ****
--- 184,191 ----
   * they not change the value of "errno" before reaching dopr().
   */

+ #ifndef SNPRINTF_VALIDATE
+
  int
  pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args)
  {
*************** pg_printf(const char *fmt,...)
*** 290,295 ****
--- 306,335 ----
      return len;
  }

+ #else                            /* SNPRINTF_VALIDATE */
+
+ /*
+  * Returns 1 if "fmt" is syntactically valid, -1 if not.
+  * In the latter case, errno is set (but it'll always be EINVAL).
+  */
+ int
+ pg_validate_printf_format(const char *fmt)
+ {
+     /* Code is basically the same as pg_vsnprintf with NULL str argument */
+     PrintfTarget target;
+     char        onebyte[1];
+
+     target.bufstart = target.bufptr = onebyte;
+     target.bufend = onebyte;
+     target.stream = NULL;
+     target.nchars = 0;
+     target.failed = false;
+     dopr(&target, fmt, NULL);
+     return target.failed ? -1 : 1;
+ }
+
+ #endif                            /* SNPRINTF_VALIDATE */
+
  /*
   * Attempt to write the entire buffer to target->stream; discard the entire
   * buffer in any case.  Call this only when target->stream is defined.
*************** dopr(PrintfTarget *target, const char *f
*** 438,445 ****
--- 478,489 ----
          if (*format == 's')
          {
              format++;
+ #ifndef SNPRINTF_VALIDATE
              strvalue = va_arg(args, char *);
              Assert(strvalue != NULL);
+ #else
+             strvalue = "";
+ #endif
              dostr(strvalue, strlen(strvalue), target);
              if (target->failed)
                  break;
*************** nextch2:
*** 666,677 ****
--- 710,726 ----
                      else
                          fieldwidth = accum;
                  }
+ #ifndef SNPRINTF_VALIDATE
                  if (have_dollar)
                      strvalue = argvalues[fmtpos].cptr;
                  else
                      strvalue = va_arg(args, char *);
                  /* Whine if someone tries to print a NULL string */
                  Assert(strvalue != NULL);
+ #else
+                 /* va_arg would return NULL, so inject empty string instead */
+                 strvalue = "";
+ #endif
                  fmtstr(strvalue, leftjust, fieldwidth, precision, pointflag,
                         target);
                  break;
*************** fail:
*** 1252,1257 ****
--- 1301,1308 ----
      target->failed = true;
  }

+ #ifndef SNPRINTF_VALIDATE
+
  /*
   * Nonstandard entry point to print a double value efficiently.
   *
*************** fail:
*** 1352,1357 ****
--- 1403,1410 ----
                                   + target.nchars);
  }

+ #endif                            /* SNPRINTF_VALIDATE */
+

  static void
  dostr(const char *str, int slen, PrintfTarget *target)
diff --git a/src/port/test_po_file.c b/src/port/test_po_file.c
index ...8dc063c .
*** a/src/port/test_po_file.c
--- b/src/port/test_po_file.c
***************
*** 0 ****
--- 1,150 ----
+ /*-------------------------------------------------------------------------
+  *
+  * test_po_file.c
+  *      Test messages in a .po file to see if our version of snprintf likes them.
+  *
+  * Copyright (c) 2018, PostgreSQL Global Development Group
+  *
+  *
+  * IDENTIFICATION
+  *      src/port/test_po_file.c
+  *
+  *-------------------------------------------------------------------------
+  */
+
+ #include "c.h"
+
+ #include <gettext-po.h>
+
+ /* snprintf.c will provide this below */
+ extern int    pg_validate_printf_format(const char *fmt);
+
+
+ /* Error handlers required by recent libgettextpo versions */
+ static void
+ xerror(int severity,
+        po_message_t message,
+        const char *filename, size_t lineno, size_t column,
+        int multiline_p, const char *message_text)
+ {
+     fprintf(stderr, "PO error: %s\n", message_text);
+     if (filename)
+         fprintf(stderr, "Error occurred in file \"%s\" at line %d:\n",
+                 filename, (int) lineno);
+     if (severity == PO_SEVERITY_FATAL_ERROR)
+         exit(1);
+ }
+
+ static void
+ xerror2(int severity,
+         po_message_t message1,
+         const char *filename1, size_t lineno1, size_t column1,
+         int multiline_p1, const char *message_text1,
+         po_message_t message2,
+         const char *filename2, size_t lineno2, size_t column2,
+         int multiline_p2, const char *message_text2)
+ {
+     fprintf(stderr, "PO error: %s ...\n", message_text1);
+     if (filename1)
+         fprintf(stderr, "Error occurred in file \"%s\" at line %d:\n",
+                 filename1, (int) lineno1);
+     fprintf(stderr, "PO error: ... %s\n", message_text2);
+     if (filename2)
+         fprintf(stderr, "Error occurred in file \"%s\" at line %d:\n",
+                 filename2, (int) lineno2);
+     if (severity == PO_SEVERITY_FATAL_ERROR)
+         exit(1);
+ }
+
+ static const struct po_xerror_handler handler = {
+     xerror, xerror2
+ };
+
+
+ /*
+  * Main program.
+  *
+  * Test each .po file named on the command line.
+  */
+ int
+ main(int argc, char **argv)
+ {
+     for (int i = 1; i < argc; i++)
+     {
+         const char *filename = argv[i];
+         po_file_t    file = po_file_read(filename, &handler);
+         const char *const *domains;
+         const char *const *domainp;
+
+         if (file == NULL)
+         {
+             fprintf(stderr, "could not open file \"%s\": %s\n",
+                     filename, strerror(errno));
+             exit(1);
+         }
+
+         domains = po_file_domains(file);
+         for (domainp = domains; *domainp; domainp++)
+         {
+             const char *domain = *domainp;
+             po_message_iterator_t iterator = po_message_iterator(file, domain);
+
+             for (;;)
+             {
+                 po_message_t message = po_next_message(iterator);
+                 po_filepos_t pos;
+                 char        location[256];
+                 const char *fmt;
+
+                 if (message == NULL)
+                     break;
+
+                 if (!po_message_is_format(message, "c-format"))
+                     continue;
+
+                 pos = po_message_filepos(message, 0);
+                 if (pos)
+                     snprintf(location, sizeof(location), "for %s:%ld",
+                              po_filepos_file(pos),
+                              (long) po_filepos_start_line(pos));
+                 else
+                     strcpy(location, "unknown location");
+
+                 fmt = po_message_msgid(message);
+                 if (fmt && pg_validate_printf_format(fmt) < 0)
+                     fprintf(stderr, "invalid format string in file \"%s\" %s\n\"%s\"\n",
+                             filename, location, fmt);
+                 fmt = po_message_msgid_plural(message);
+                 if (fmt && pg_validate_printf_format(fmt) < 0)
+                     fprintf(stderr, "invalid format string in file \"%s\" %s\n\"%s\"\n",
+                             filename, location, fmt);
+                 fmt = po_message_msgstr(message);
+                 if (fmt && pg_validate_printf_format(fmt) < 0)
+                     fprintf(stderr, "invalid format string in file \"%s\" %s\n\"%s\"\n",
+                             filename, location, fmt);
+                 for (int ndx = 0;; ndx++)
+                 {
+                     fmt = po_message_msgstr_plural(message, ndx);
+                     if (!fmt)
+                         break;
+                     if (pg_validate_printf_format(fmt) < 0)
+                         fprintf(stderr, "invalid format string in file \"%s\" %s\n\"%s\"\n",
+                                 filename, location, fmt);
+                 }
+             }
+             po_message_iterator_free(iterator);
+         }
+         po_file_free(file);
+     }
+
+     return 0;
+ }
+
+ /*
+  * Now include snprintf.c with the desired variant compilation flag.
+  * We put this at the end so that its messing about with #define's
+  * won't mess up code in this file.
+  */
+ #define SNPRINTF_VALIDATE 1
+
+ #include "snprintf.c"

On 07/12/2018 00:38, Tom Lane wrote:
> ... so my preliminary result is that there's a *boatload* of "invalid"
> format strings in our translation files, all of them apparently with
> the extra-space disease.  See attached.

A bit of follow-up:  I have committed fixes for all the erroneous uses
of space "flags" in translated format strings.  These will then be in
the next minor releases.

I also looked into the gettext/msgfmt source code.  We know that it does
check for format string compatibility, but I found that it only checks
the general type, not the formatting flags.  I suppose this is not
unreasonable, since a translation might want to space or align something
differently.  So we still don't have an easy way to check for this in
the future.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services