On Thu, Feb 28, 2019 at 04:32:44PM +0000, Elliott, Robert (Persistent Memory) wrote: > > -----Original Message----- > > From: Grub-devel <grub-devel-bounces+elliott=hpe....@gnu.org> On > > Behalf Of Colin Watson > ... > > -void > > +int > > grub_util_file_sync (FILE *f) > > { > > - fflush (f); > > + if (fflush (f) != 0) > > + return -1; > > if (!allow_fd_syncs) > > - return; > > - fsync (fileno (f)); > > + return 0; > > + return fsync (fileno (f)); > > } > > Since that's just returning -1 for an error (both for fflush and fsync), > not errno which contains the reason for the error... > > > diff --git a/util/editenv.c b/util/editenv.c > > index c6f8d2298..eb2d0c03a 100644 > > --- a/util/editenv.c > > +++ b/util/editenv.c > > @@ -55,7 +55,8 @@ grub_util_create_envblk_file (const char *name) > > strerror (errno)); > > > > > > - grub_util_file_sync (fp); > > + if (grub_util_file_sync (fp) < 0) > > + grub_util_error (_("cannot sync `%s': %s"), namenew, strerror > > (errno)); > > callers like this will interpret the -1 as EPERM, which isn't the > true reason.
No, this is a mistaken analysis. The code you're quoting passes errno to strerror when rendering the error message, *not* the return code of grub_util_file_sync. It is therefore not possible for it to misinterpret -1 as EPERM. The same holds for all similar call sites in my patch. (In any case, the value of EPERM is typically 1, not -1; you may be thinking of the kernel practice of returning -errno, which is not common in userspace code.) The pattern I've followed for grub_util_file_sync and other similar functions is the commonplace one used by many C library functions: it either returns 0, or it returns some other value (in this case -1) and sets errno to indicate the error. It is of course necessary to ensure that it only returns -1 when something has already set errno, but I believe that's the case throughout my patch. -- Colin Watson [cjwat...@ubuntu.com] _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel