[PATCH] gropdf: Fix papersize control command

[Alexis]

Hi folks,

I may have stumbled upon a bug related to gropdf's papersize control
command. Please note that my understanding of how things are supposed
to work may very well be too limited and lead me to false assumptions.

In brief: The \X'papersize=paper-format' control command seems to
read the paper-format parameter as width,length as opposed to the
gropdf -p option and the referenced documentation (groff_font(5))
which specifies length,width for the paper-format argument.


In detail: The explanation for the \X'papersize=paper-format' control
command in gropdf(1) states that the paper-format parameter is the same
as the papersize directive in groff_font(5), which states that "the
argument can be a custom paper format length,width".

Given the -p2i,1i option gropdf creates a page that is 144 pt tall
and 72 pt wide as expected (note that pdfinfo reports the Page size as
width x height pts):

  %  echo "\\&" | groff -Tpdf -P-p2i,1i | pdfinfo - | grep ^Page[^s]
  Page size:       72 x 144 pts
  Page rot:        0

Given the \X'papersize=2i,1i' control command I'd expect the same
result, yet gropdf creates a page that is 72 pt tall and 144 pt wide:

  % echo "\\&\\X'papersize=2i,1i'" | groff -Tpdf | pdfinfo - | grep ^Page[^s]
  Page size:       144 x 72 pts
  Page rot:        0

If my reading of the relevant code section in gropdf is correct:

  % grep -nA5 "'papersize'" src/devices/gropdf/gropdf.pl
  1435:   elsif (lc(substr($xprm[0],0,9)) eq 'papersize')
  1436-   {
  1437-       my ($px,$py)=split(',',substr($xprm[0],10));
  1438-       $px=GetPoints($px);
  1439-       $py=GetPoints($py);
  1440-       @mediabox=(0,0,$px,$py);

then gropdf parses the papersize command-control paper-format parameter
as width,length as the *first* result of split is assigned to $px,
which is then passed as the third parameter to @mediabox. The code
section in gropdf handling the -p option:

  % grep -nA3 "Allow height" src/devices/gropdf/gropdf.pl
  324:    # Allow height,width specified directly in centimeters, inches, or 
points.
  325-    if ($papersz=~m/([\d.]+)([cipP]),([\d.]+)([cipP])/)
  326-    {
  327-        @defaultmb=@mediabox=(0,0,ToPoints($3,$4),ToPoints($1,$2));

however swaps the regex match group pairs passing the *second*
match group pair ($3,$4) to ToPoints and passing that result as the
third parameter to @mediabox.

Furthermore the gropdf(1) manpage mentions that the papersize control
command is also used by the DVI driver. Inspecting the grodvi code:

  % grep -nB3 "paper format" src/devices/grodvi/dvi.cpp
  936-    case 'p':
  937-      if (!font::scan_papersize(optarg, 0,
  938-                            &user_paper_length, &user_paper_width))
  939:    error("ignoring invalid paper format '%1'", optarg);

the font::scan_papersize function is used to parse the -p option
arguments, which seems to use sscanf(3) to parse the given argument:

  % sed -n '/::scan_papersize/,/sscanf/p' src/libs/libgroff/font.cpp
  bool font::scan_papersize(const char *p, const char **size,
                            double *length, double *width)
  {
    double l, w;
    char lu[2], wu[2];
    const char *pp = p;
    bool attempt_file_open = true;
    char line[255];
  again:
    if (csdigit(*pp)) {
      if (sscanf(pp, "%lf%1[ipPc],%lf%1[ipPc]", &l, lu, &w, wu) == 4

Unless my Perl, PCRE, C "fluency" is failing me entirely then the -p
option argument is parsed as length,width in accordance with the
documentation.

If you agree with my findings and reasoning and what I have stumbled
upon is indeed a bug please find attached a patch for gropdf that
addresses the issue and also bringing the papersize control command
argument handling code a bit more in line with the code handling
the -p option argument.

Should my current understanding of groff and the implementation have
lead me astray to falsely believe there is an issue where in fact
things work the way they are supposed to I'd appreciate explanations
and/or pointers to help me deepen my understanding.


Best
Alexis

gropdf_papersize.patch
Description: Text document