I don't see the point of this, and think you are cargo culting.
Shall we change all the fopen() calls in src/bin to use the same
idiom? No. What happens is you point at a directory, and either
read zero bytes or garbage, and you get what you asked for.
So I believe this function should *not exist*, but instead you
think we should embrace it so that pfctl can split out 3 error
messages in a row??
> There's pfctl_fopen() since 2004, but it's not been used consistently:
>
> # pfctl -f/ && pfctl -sr ; echo $?
> 0
>
> Diff below converts all three fopen(2) callers to pfctl_fopen(),
> see inlined for easier review:
>
> FILE *
> pfctl_fopen(const char *name, const char *mode)
> {
> struct stat st;
> FILE *fp;
>
> fp = fopen(name, mode);
> if (fp == NULL)
> return (NULL);
> if (fstat(fileno(fp), &st)) {
> fclose(fp);
> return (NULL);
> }
> if (S_ISDIR(st.st_mode)) {
> fclose(fp);
> errno = EISDIR;
> return (NULL);
> }
> return (fp);
> }
>
> # ./obj/pfctl -f/
> pfctl: pushfile: /: Is a directory
> pfctl: cannot open the main config file!: Is a directory
> pfctl: Syntax error in config file: pf rules not loaded
>
> Not pretty but safe. OK?
>
> Index: parse.y
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/parse.y,v
> retrieving revision 1.695
> diff -u -p -r1.695 parse.y
> --- parse.y 18 Apr 2019 21:58:59 -0000 1.695
> +++ parse.y 19 Apr 2019 13:00:24 -0000
> @@ -5476,7 +5476,7 @@ pushfile(const char *name, int secret)
> free(nfile);
> return (NULL);
> }
> - } else if ((nfile->stream = fopen(nfile->name, "r")) == NULL) {
> + } else if ((nfile->stream = pfctl_fopen(nfile->name, "r")) == NULL) {
> warn("%s: %s", __func__, nfile->name);
> free(nfile->name);
> free(nfile);
> Index: pfctl.c
> ===================================================================
> RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
> retrieving revision 1.373
> diff -u -p -r1.373 pfctl.c
> --- pfctl.c 15 Apr 2019 21:36:44 -0000 1.373
> +++ pfctl.c 19 Apr 2019 13:00:24 -0000
> @@ -2173,9 +2173,8 @@ pfctl_state_store(int dev, const char *f
> char *inbuf = NULL, *newinbuf = NULL;
> size_t n, len = 0;
>
> - f = fopen(file, "w");
> - if (f == NULL)
> - err(1, "open: %s", file);
> + if ((f = pfctl_fopen(file, "w")) == NULL)
> + err(1, "pfctl_fopen: %s", file);
>
> memset(&ps, 0, sizeof(ps));
> for (;;) {
> @@ -2215,9 +2214,8 @@ pfctl_state_load(int dev, const char *fi
> FILE *f;
> struct pfioc_state ps;
>
> - f = fopen(file, "r");
> - if (f == NULL)
> - err(1, "open: %s", file);
> + if ((f = pfctl_fopen(file, "r")) == NULL)
> + err(1, "pfctl_fopen: %s", file);
>
> while (fread(&ps.state, sizeof(ps.state), 1, f) == 1) {
> if (ioctl(dev, DIOCADDSTATE, &ps) < 0) {
>
