On Fri, Mar 03, 2017 at 05:06:57PM +0700, Duy Nguyen wrote:
> > But if we do, I think we'd either want to:
> >
> > a. actually check ferror() after getting EOF and report the read
> > error. That catches EISDIR, along with any other unexpected
> > errors.
> >
> > b. use an fopen wrapper that checks fstat(fileno(fh)) after the
> > open, and turns fopen(some_dir) into an error.
>
> If you don't like extra check, I guess you're negative on b as well
> since it is an extra check on Windows. That leaves us with option a.
I don't mind _doing_ the extra check that much. I don't think we fopen
so many files that an extra fstat on each would kill us. I mostly just
don't like having to sprinkle the explicit call to it everywhere. I'd be
OK with:
FILE *xfopen(const char *path, const char *mode)
{
FILE *ret = fopen(path, mode);
#ifdef FOPEN_OPENS_DIRECTORIES
if (ret) {
struct stat st;
if (!fstat(fileno(ret), &st) && S_ISDIR(st.st_mode)) {
fclose(ret);
ret = NULL;
}
}
#endif
return ret;
}
But I do think option (a) is cleaner. The only trick is that for errno
to be valid, we need to make sure we check ferror() soon after seeing
the EOF return value. I suspect it would work OK in practice for the
git_config_from_file() case.
-Peff