Re: [PATCH] util: Detect more I/O errors
On Wed, Feb 27, 2019 at 09:10:08AM +, Colin Watson wrote: >Many of GRUB's utilities don't check anywhere near all the possible >write errors. For example, if grub-install runs out of space when >copying a file, it won't notice. There were missing checks for the >return values of write, fflush, fsync, and close (or the equivalents on >other OSes), all of which must be checked. > >I tried to be consistent with the existing logging practices of the >various hostdisk implementations, but they weren't entirely consistent >to start with so I used my judgement. The result at least looks >reasonable on GNU/Linux when I provoke a write error: > > Installing for x86_64-efi platform. > grub-install: error: cannot copy > `/usr/lib/grub/x86_64-efi-signed/grubx64.efi.signed' to > `/boot/efi/EFI/debian/grubx64.efi': No space left on device. > >There are more missing checks in other utilities, but this should fix >the most critical ones. > >Fixes Debian bug #922741. >--- > grub-core/osdep/aros/hostdisk.c| 38 ++ > grub-core/osdep/unix/hostdisk.c| 18 +++--- > grub-core/osdep/windows/hostdisk.c | 37 ++--- > include/grub/emu/hostfile.h| 4 ++-- > include/grub/emu/misc.h| 2 +- > util/editenv.c | 3 ++- > util/grub-editenv.c| 3 ++- > util/grub-install-common.c | 16 + > util/grub-mkimage.c| 8 +-- > util/setup.c | 6 +++-- > 10 files changed, 90 insertions(+), 45 deletions(-) LGTM Reviewed-by: Steve McIntyre <93...@debian.org> > >diff --git a/grub-core/osdep/aros/hostdisk.c b/grub-core/osdep/aros/hostdisk.c >index 7d99b54b8..2be654ca3 100644 >--- a/grub-core/osdep/aros/hostdisk.c >+++ b/grub-core/osdep/aros/hostdisk.c >@@ -439,36 +439,44 @@ grub_util_get_fd_size (grub_util_fd_t fd, > return -1; > } > >-void >+int > grub_util_fd_close (grub_util_fd_t fd) > { > switch (fd->type) > { > case GRUB_UTIL_FD_FILE: >- close (fd->fd); >- return; >+ return close (fd->fd); > case GRUB_UTIL_FD_DISK: > CloseDevice ((struct IORequest *) fd->ioreq); > DeleteIORequest((struct IORequest *) fd->ioreq); > DeleteMsgPort (fd->mp); >- return; >+ return 0; > } >+ return 0; > } > > static int allow_fd_syncs = 1; > >-static void >+static int > grub_util_fd_sync_volume (grub_util_fd_t fd) > { >+ LONG err; >+ > fd->ioreq->iotd_Req.io_Command = CMD_UPDATE; > fd->ioreq->iotd_Req.io_Length = 0; > fd->ioreq->iotd_Req.io_Data = 0; > fd->ioreq->iotd_Req.io_Offset = 0; > fd->ioreq->iotd_Req.io_Actual = 0; >- DoIO ((struct IORequest *) fd->ioreq); >+ err = DoIO ((struct IORequest *) fd->ioreq); >+ if (err) >+{ >+ grub_util_info ("I/O failed with error %d, IoErr=%d", (int)err, (int) >IoErr ()); >+ return -1; >+} >+ return 0; > } > >-void >+int > grub_util_fd_sync (grub_util_fd_t fd) > { > if (allow_fd_syncs) >@@ -476,22 +484,22 @@ grub_util_fd_sync (grub_util_fd_t fd) > switch (fd->type) > { > case GRUB_UTIL_FD_FILE: >-fsync (fd->fd); >-return; >+return fsync (fd->fd); > case GRUB_UTIL_FD_DISK: >-grub_util_fd_sync_volume (fd); >-return; >+return grub_util_fd_sync_volume (fd); > } > } >+ return 0; > } > >-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)); > } > > void >diff --git a/grub-core/osdep/unix/hostdisk.c b/grub-core/osdep/unix/hostdisk.c >index 5450cf416..91150969b 100644 >--- a/grub-core/osdep/unix/hostdisk.c >+++ b/grub-core/osdep/unix/hostdisk.c >@@ -177,20 +177,22 @@ grub_util_fd_strerror (void) > > static int allow_fd_syncs = 1; > >-void >+int > grub_util_fd_sync (grub_util_fd_t fd) > { > if (allow_fd_syncs) >-fsync (fd); >+return fsync (fd); >+ return 0; > } > >-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)); > } > > void >@@ -199,10 +201,10 @@ grub_util_disable_fd_syncs (void) > allow_fd_syncs = 0; > } > >-void >+int > grub_util_fd_close (grub_util_fd_t fd) > { >- close (fd); >+ return close (fd); > } > > char * >diff --git a/grub-core/osdep/windows/hostdisk.c >b/grub-core/osdep/windows/hostdisk.c >index 85507af88..355100789 100644 >--- a/grub-core/osdep/windows/hostdisk.c >+++ b/grub-core/osdep/windows/hostdisk.c >@@ -275,11 +275,18 @@ grub_util_fd_write (grub_util_fd_t fd, const char *buf, >size_t len) > > static int allow_fd_syncs = 1; > >-void >+int > grub_util_fd_sync (grub_util_fd_t fd) > { > if (allow_fd_syncs) >-FlushFileBuffers (fd); >+{ >+ if (!FlushFileBuffers (fd)) >+ { >+
[PATCH v2] Fix syslinux_test in out-of-tree builds
syslinux_parse simplifies some filenames by removing things like ".." segments, but the tests assumed that @abs_top_srcdir@ would be untouched, which is not true in the case of out-of-tree builds where @abs_top_srcdir@ may contain ".." segments. Performing the substitution requires some awkwardness in Makefile.am due to details of how config.status works. Signed-off-by: Colin Watson --- Makefile.am| 7 ++- grub-core/lib/syslinux_parse.c | 3 +++ tests/syslinux/ubuntu10.04_grub.cfg.in | 20 ++-- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/Makefile.am b/Makefile.am index a52a998a1..5b3cf5b4d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -476,6 +476,11 @@ EXTRA_DIST += ChangeLog ChangeLog-2015 syslinux_test: $(top_builddir)/config.status tests/syslinux/ubuntu10.04_grub.cfg +# Mimic simplify_filename from grub-core/lib/syslinux_parse.c, so that we +# can predict its behaviour in tests. We have to pre-substitute this before +# calling config.status, as config.status offers no reliable way to hook in +# a command between setting ac_abs_top_srcdir and emitting output files. tests/syslinux/ubuntu10.04_grub.cfg: $(top_builddir)/config.status tests/syslinux/ubuntu10.04_grub.cfg.in - (for x in tests/syslinux/ubuntu10.04_grub.cfg.in ; do cat $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:- + simplified_abs_top_srcdir=`echo "$(abs_top_srcdir)" | sed 's,//,/,g; s,/\./,/,g; :loop; s,/[^/][^/]*/\.\.\(/\|$$\),\1,; t loop'`; \ + sed "s,@simplified_abs_top_srcdir@,$$simplified_abs_top_srcdir,g" $(srcdir)/tests/syslinux/ubuntu10.04_grub.cfg.in | $(top_builddir)/config.status --file=$@:- CLEANFILES += tests/syslinux/ubuntu10.04_grub.cfg diff --git a/grub-core/lib/syslinux_parse.c b/grub-core/lib/syslinux_parse.c index c96d85ee5..c117c584d 100644 --- a/grub-core/lib/syslinux_parse.c +++ b/grub-core/lib/syslinux_parse.c @@ -807,6 +807,9 @@ print_file (struct output_buffer *outbuf, return print_escaped (outbuf, from, to); } +/* Makefile.am mimics this when generating + tests/syslinux/ubuntu10.04_grub.cfg, so changes here may need to be + reflected there too. */ static void simplify_filename (char *str) { diff --git a/tests/syslinux/ubuntu10.04_grub.cfg.in b/tests/syslinux/ubuntu10.04_grub.cfg.in index 846e4acf0..441dec045 100644 --- a/tests/syslinux/ubuntu10.04_grub.cfg.in +++ b/tests/syslinux/ubuntu10.04_grub.cfg.in @@ -41,7 +41,7 @@ menuentry 'Test memory' --hotkey 'm' --id 'memtest' { linux$linux_suffix '/'/'/install/mt86plus' } menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' { -# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/gtk.cfg not found +# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/gtk.cfg not found # UNSUPPORTED command 'menu begin advanced' # UNSUPPORTED command 'menu title Advanced options' # UNSUPPORTED command 'menu color title * # *' @@ -63,14 +63,14 @@ menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' { } menuentry 'Back..' --hotkey 'b' --id 'mainmenu' { # UNSUPPORTED command 'menu exit' -# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/adgtk.cfg not found +# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/adgtk.cfg not found # UNSUPPORTED command 'menu end' # UNSUPPORTED entry type 0 true; } menuentry 'Help' --hotkey 'h' --id 'help' { # UNSUPPORTED command 'ui gfxboot bootlogo' -#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux'/'prompt.cfg' (host)@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg: +#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux'/'prompt.cfg' (host)@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg: background_image '@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/'/'splash.png' # UNSUPPORTED command 'display f1.txt' # UNSUPPORTED command 'menu hshift 13' @@ -114,7 +114,7 @@ menuentry 'Test memory' --hotkey 'm' --id 'memtest' { linux$linux_suffix '/'/'/install/mt86plus' } menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' { -# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//gtk.cfg not found +# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//gtk.cfg not found # UNSUPPORTED command 'menu begin advanced' # UNSUPPORTED command 'menu title Advanced options' # UNSUPPORTED command 'menu color title * # *' @@ -136,13 +136,13 @@ menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' { } menuentry 'Back..' --hotkey 'b' --id 'mainmenu' { # UNSUPPORTED command 'menu exit' -# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//adgtk.cfg not found +# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//adgtk.cfg not found # UNSUPPORTED command 'menu end' # UNSUPPORTED entry type 0
Re: [PATCH] Fix syslinux_test in out-of-tree builds
On Mon, Jan 14, 2019 at 02:15:14PM +0100, Daniel Kiper wrote: > On Wed, Jan 09, 2019 at 02:59:12PM +, Colin Watson wrote: > > +# Mimic simplify_filename from grub-core/lib/syslinux_parse.c, so that we > > OK, but I would like to see a comment before > grub-core/lib/syslinux_parse.c:simplify_filename() saying that somebody > changing its code should take care of the code here too. Otherwise > sooner or later the tests will be broken again due oversight. Fair enough. > > +# can predict its behaviour in tests. We have to pre-substitute this > > before > > +# calling config.status, as config.status offers no reliable way to hook in > > +# a command between setting ac_abs_top_srcdir and emitting output files. > > tests/syslinux/ubuntu10.04_grub.cfg: $(top_builddir)/config.status > > tests/syslinux/ubuntu10.04_grub.cfg.in > > - (for x in tests/syslinux/ubuntu10.04_grub.cfg.in ; do cat > > $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:- > > + simplified_abs_top_srcdir=`echo "$(abs_top_srcdir)" | sed 's,//,/,g; > > s,/\./,/,g; :loop; s,/[^/][^/]*/\.\.\(/\|$$\),\1,; t loop'`; \ > > + (for x in tests/syslinux/ubuntu10.04_grub.cfg.in ; do sed > > "s,@simplified_abs_top_srcdir@,$$simplified_abs_top_srcdir,g" > > $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:- > > I think that you can drop this for. Done. > > menuentry 'Help' --hotkey 'h' --id 'help' { > ># UNSUPPORTED command 'ui gfxboot bootlogo' > > -#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//'/'prompt.cfg' > > (host)@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg: > > +#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//'/'prompt.cfg' > > (host)@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg: > Hmmm... Why number of "/" increases from top to down ---> I guess sometimes get_target_filename is called with an empty string or something and it doesn't bother to simplify that? This wasn't introduced by my patch, so I haven't worked out the exact code paths involved. I'll send a v2. -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2 0/1] Upgrade Gnulib; switch to bootstrap tool
I have a bug report that I think is best handled by importing and using a new module from Gnulib, so I tried to do that. Unfortunately GRUB's Gnulib import is in a confusing state that makes it difficult to do this. The last full update was in 2013, without documentation of the exact commit that was imported. Since then it's been maintained by various ad-hoc hacks and importing new modules has become difficult. Other GNU projects use a "bootstrap" tool that's been added to Gnulib, and I think that's the best thing to do here as well. Since GRUB's autogen.sh does substantially more than just calling autoreconf, I've retained it so that it can be used by downstream packages that need to patch and regenerate GRUB's build system without reimporting Gnulib. This patch is necessarily very large because it removes many autogenerated files. For ease of review, here's a version of the patch without those removals, produced by running it through this command: filterdiff -p1 \ -x ABOUT-NLS -x build-aux/\* -x grub-core/gnulib/\* -x m4/\* \ -x po/\* Note that the added "bootstrap" file is a verbatim copy of gnulib/build-aux/bootstrap at the commit identified in bootstrap.conf. [v2: Update INSTALL and docs/grub-dev.texi; move grub-core/gnulib/ to grub-core/lib/gnulib/; move patches to grub-core/lib/gnulib-patches/.] diff --git a/.gitignore b/.gitignore index eca17bec9..819cd185d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ +*~ 00_header 10_* 20_linux_xen @@ -6,11 +7,13 @@ 41_custom *.1 *.8 +ABOUT-NLS aclocal.m4 ahci_test ascii.bitmaps ascii.h autom4te.cache +build-aux build-grub-gen-asciih build-grub-gen-widthspec build-grub-mkfont @@ -42,6 +45,7 @@ gensymlist.sh gentrigtables gentrigtables.exe gettext_strings_test +/gnulib grub-bin2h /grub-bios-setup /grub-bios-setup.exe @@ -134,6 +138,7 @@ help_test *.image.exe include/grub/cpu include/grub/machine +INSTALL.grub install-sh lib/libgcrypt-grub libgrub_a_init.c @@ -142,6 +147,7 @@ libgrub_a_init.c lzocompress_test *.marker Makefile +/m4 *.mod mod-*.c missing @@ -155,7 +161,11 @@ pata_test *.pp po/*.mo po/grub.pot +po/Makefile.in.in +po/Makevars +po/Makevars.template po/POTFILES +po/Rules-quot po/stamp-po printf_test priority_queue_unit_test @@ -202,25 +212,7 @@ grub-core/*.module.exe grub-core/*.pp grub-core/kernel.img.bin util/bash-completion.d/grub -grub-core/gnulib/alloca.h -grub-core/gnulib/arg-nonnull.h -grub-core/gnulib/c++defs.h -grub-core/gnulib/charset.alias -grub-core/gnulib/configmake.h -grub-core/gnulib/float.h -grub-core/gnulib/getopt.h -grub-core/gnulib/langinfo.h -grub-core/gnulib/ref-add.sed -grub-core/gnulib/ref-del.sed -grub-core/gnulib/stdio.h -grub-core/gnulib/stdlib.h -grub-core/gnulib/string.h -grub-core/gnulib/strings.h -grub-core/gnulib/sys -grub-core/gnulib/unistd.h -grub-core/gnulib/warn-on-use.h -grub-core/gnulib/wchar.h -grub-core/gnulib/wctype.h +grub-core/lib/gnulib grub-core/rs_decoder.h widthspec.bin widthspec.h @@ -239,10 +231,6 @@ po/POTFILES-shell.in /grub-render-label /grub-glue-efi.exe /grub-render-label.exe -grub-core/gnulib/locale.h -grub-core/gnulib/unitypes.h -grub-core/gnulib/uniwidth.h -build-aux/test-driver /garbage-gen /garbage-gen.exe /grub-fs-tester diff --git a/INSTALL b/INSTALL index b370d7753..6527c8e7c 100644 --- a/INSTALL +++ b/INSTALL @@ -102,10 +102,11 @@ The simplest way to compile this package is: 2. Skip this and following step if you use release tarball and proceed to step 4. If you want translations type `./linguas.sh'. - 3. Type `./autogen.sh'. + 3. Type `./bootstrap'. - * autogen.sh uses python. By default invocation is "python" but can be - overriden by setting variable $PYTHON. + * autogen.sh (called by bootstrap) uses python. By default the + invocation is "python", but it can be overridden by setting the + variable $PYTHON. 4. Type `./configure' to configure the package for your system. If you're using `csh' on an old version of System V, you might diff --git a/Makefile.am b/Makefile.am index a52a998a1..3f18aecbb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,7 +1,7 @@ AUTOMAKE_OPTIONS = subdir-objects -Wno-portability DEPDIR = .deps-util -SUBDIRS = grub-core/gnulib . +SUBDIRS = grub-core/lib/gnulib . if COND_real_platform SUBDIRS += grub-core endif diff --git a/Makefile.util.def b/Makefile.util.def index a0495a87f..969d32f00 100644 --- a/Makefile.util.def +++ b/Makefile.util.def @@ -198,7 +198,7 @@ program = { ldadd = libgrubmods.a; ldadd = libgrubgcry.a; ldadd = libgrubkern.a; - ldadd = grub-core/gnulib/libgnu.a; + ldadd = grub-core/lib/gnulib/libgnu.a; ldadd = '$(LIBLZMA)'; ldadd = '$(LIBINTL) $(LIBDEVMAPPER) $(LIBZFS) $(LIBNVPAIR) $(LIBGEOM)'; cppflags = '-DGRUB_PKGLIBDIR=\"$(pkglibdir)\"'; @@ -215,7 +215,7 @@ program = { ldadd = libgrubmods.a; ldadd = libgrubgcry.a; ldadd = libgrubkern.a; - ldadd = grub-core/gnulib/libgnu.a; +
Re: [PATCH 1/1] Upgrade Gnulib; switch to bootstrap tool
(Sorry for the slow response.) On Wed, Jan 16, 2019 at 09:03:40PM +0100, Daniel Kiper wrote: > Could you update INSTALL file too? Done. > Additionally, if you do such huge change could you move Gnulib from > grub-core/gnulib to grub-core/lib/gnulib? Ending up with grub-core/lib/gnulib/libgnu.a seems pretty repetitive, but on the other hand it does keep gnulib together with other (semi-)autogenerated things like libgcrypt, so OK. > Including all diffs which are currently outside of grub-core/gnulib > directory. Well, maybe *.diff files should be renamed to *.patch... These can't be in grub-core/lib/gnulib/; that whole directory should be regarded as being wiped out by gnulib-tool (it may not in fact be, but in my experience it's best to keep manually-maintained files separate from the directory that you give to gnulib-tool --source-base=). I agree that they should be collected in their own directory though. I've moved them to grub-core/lib/gnulib-patches/ instead, and renamed to .patch as you suggest. > Could you add to the commit message how to exactly update Gnulib? > Just list of steps like in the commit 461f1d8af (Import upstream > zstd-1.3.6). Commit messages aren't a good place for this sort of thing. (I mean, they're better than nothing, but they're still not great.) I've updated docs/grub-dev.texi instead, adding a "Gnulib" section under a new "Updating external code" chapter. I hope this will encourage people who know about the other bits of external code that GRUB imports to add corresponding sections there. I'll send a v2. Thanks, -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] util: Detect more I/O errors
Many of GRUB's utilities don't check anywhere near all the possible write errors. For example, if grub-install runs out of space when copying a file, it won't notice. There were missing checks for the return values of write, fflush, fsync, and close (or the equivalents on other OSes), all of which must be checked. I tried to be consistent with the existing logging practices of the various hostdisk implementations, but they weren't entirely consistent to start with so I used my judgement. The result at least looks reasonable on GNU/Linux when I provoke a write error: Installing for x86_64-efi platform. grub-install: error: cannot copy `/usr/lib/grub/x86_64-efi-signed/grubx64.efi.signed' to `/boot/efi/EFI/debian/grubx64.efi': No space left on device. There are more missing checks in other utilities, but this should fix the most critical ones. Fixes Debian bug #922741. --- grub-core/osdep/aros/hostdisk.c| 38 ++ grub-core/osdep/unix/hostdisk.c| 18 +++--- grub-core/osdep/windows/hostdisk.c | 37 ++--- include/grub/emu/hostfile.h| 4 ++-- include/grub/emu/misc.h| 2 +- util/editenv.c | 3 ++- util/grub-editenv.c| 3 ++- util/grub-install-common.c | 16 + util/grub-mkimage.c| 8 +-- util/setup.c | 6 +++-- 10 files changed, 90 insertions(+), 45 deletions(-) diff --git a/grub-core/osdep/aros/hostdisk.c b/grub-core/osdep/aros/hostdisk.c index 7d99b54b8..2be654ca3 100644 --- a/grub-core/osdep/aros/hostdisk.c +++ b/grub-core/osdep/aros/hostdisk.c @@ -439,36 +439,44 @@ grub_util_get_fd_size (grub_util_fd_t fd, return -1; } -void +int grub_util_fd_close (grub_util_fd_t fd) { switch (fd->type) { case GRUB_UTIL_FD_FILE: - close (fd->fd); - return; + return close (fd->fd); case GRUB_UTIL_FD_DISK: CloseDevice ((struct IORequest *) fd->ioreq); DeleteIORequest((struct IORequest *) fd->ioreq); DeleteMsgPort (fd->mp); - return; + return 0; } + return 0; } static int allow_fd_syncs = 1; -static void +static int grub_util_fd_sync_volume (grub_util_fd_t fd) { + LONG err; + fd->ioreq->iotd_Req.io_Command = CMD_UPDATE; fd->ioreq->iotd_Req.io_Length = 0; fd->ioreq->iotd_Req.io_Data = 0; fd->ioreq->iotd_Req.io_Offset = 0; fd->ioreq->iotd_Req.io_Actual = 0; - DoIO ((struct IORequest *) fd->ioreq); + err = DoIO ((struct IORequest *) fd->ioreq); + if (err) +{ + grub_util_info ("I/O failed with error %d, IoErr=%d", (int)err, (int) IoErr ()); + return -1; +} + return 0; } -void +int grub_util_fd_sync (grub_util_fd_t fd) { if (allow_fd_syncs) @@ -476,22 +484,22 @@ grub_util_fd_sync (grub_util_fd_t fd) switch (fd->type) { case GRUB_UTIL_FD_FILE: - fsync (fd->fd); - return; + return fsync (fd->fd); case GRUB_UTIL_FD_DISK: - grub_util_fd_sync_volume (fd); - return; + return grub_util_fd_sync_volume (fd); } } + return 0; } -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)); } void diff --git a/grub-core/osdep/unix/hostdisk.c b/grub-core/osdep/unix/hostdisk.c index 5450cf416..91150969b 100644 --- a/grub-core/osdep/unix/hostdisk.c +++ b/grub-core/osdep/unix/hostdisk.c @@ -177,20 +177,22 @@ grub_util_fd_strerror (void) static int allow_fd_syncs = 1; -void +int grub_util_fd_sync (grub_util_fd_t fd) { if (allow_fd_syncs) -fsync (fd); +return fsync (fd); + return 0; } -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)); } void @@ -199,10 +201,10 @@ grub_util_disable_fd_syncs (void) allow_fd_syncs = 0; } -void +int grub_util_fd_close (grub_util_fd_t fd) { - close (fd); + return close (fd); } char * diff --git a/grub-core/osdep/windows/hostdisk.c b/grub-core/osdep/windows/hostdisk.c index 85507af88..355100789 100644 --- a/grub-core/osdep/windows/hostdisk.c +++ b/grub-core/osdep/windows/hostdisk.c @@ -275,11 +275,18 @@ grub_util_fd_write (grub_util_fd_t fd, const char *buf, size_t len) static int allow_fd_syncs = 1; -void +int grub_util_fd_sync (grub_util_fd_t fd) { if (allow_fd_syncs) -FlushFileBuffers (fd); +{ + if (!FlushFileBuffers (fd)) + { + grub_util_info ("flush err %x", (int) GetLastError ()); + return -1; + } +} + return 0; } void @@ -288,10 +295,15 @@ grub_util_disable_fd_syncs (void) allow_fd_syncs = 0; } -void +int grub_util_fd_close (grub_util_fd_t fd) { - CloseHandle (fd); + if (!CloseHandle