bdftopcf is intended to be portable code.  I don't think it is
right to start using <err.h> functions in here.  They are within
an unveil-block which we'll carry as a diff, but still.. it doesn't
feel right.  I think you should use fprintf to stderr and exit as
the existing code does.

> If input_name is provided we can unveil it with read permissions, if
> output_name is provided we need to unveil this one with rwc. Additionally
> depending on the different combinations of if these files are passed via args
> or from stdin/to stdout we can also pledge accordingly to the code path. This
> has been tested succefully with bdf fonts we have bundled in xenocara.
> 
> Since I have several other X apps unveiled and/or pledged could you please
> comment not only with the unveil/pledge part, but also err vs fprintf/exit, 
> the
> placement of the #includes and also tabs vs spaces?
> 
> Index: bdftopcf.c
> ===================================================================
> RCS file: /cvs/xenocara/app/bdftopcf/bdftopcf.c,v
> retrieving revision 1.5
> diff -u -p -u -r1.5 bdftopcf.c
> --- bdftopcf.c        29 Mar 2018 20:34:30 -0000      1.5
> +++ bdftopcf.c        24 Oct 2018 10:15:41 -0000
> @@ -38,7 +38,9 @@ from The Open Group.
>  #include "fntfil.h"
>  #include "bdfint.h"
>  #include "pcf.h"
> +#include <err.h>
>  #include <stdio.h>
> +#include <unistd.h>
>  #include <X11/Xos.h>
>  
>  int
> @@ -158,6 +160,26 @@ main(int argc, char *argv[])
>          }
>          argv++;
>      }
> +
> +     if (input_name) {
> +             if (unveil(input_name, "r") == -1)
> +                     err(1, "unveil");
> +     }
> +     if (output_name) {
> +             if (unveil(output_name, "rwc") == -1)
> +                     err(1, "unveil");
> +             if (pledge("stdio rpath wpath cpath", NULL) == -1)
> +                     err(1, "pledge");
> +     }
> +     if (input_name && !output_name) {
> +             if (pledge("stdio rpath", NULL) == -1)
> +                     err(1, "pledge");
> +     }
> +     if (!input_name && !output_name) {
> +             if (pledge("stdio", NULL) == -1)
> +                     err(1, "pledge");
> +     }
> +
>      if (input_name) {
>          input = FontFileOpen(input_name);
>          if (!input) {

Reply via email to