Thanks for the update. I'm glad to hear it made the cut.
On Sun, Nov 3, 2013 at 7:49 PM, Vladimir 'φ-coder/phcoder' Serbinenko < phco...@gmail.com> wrote: > > > > -------- Original Message -------- > Subject: Re: New command to check NT's hibernation state > Date: Mon, 04 Nov 2013 01:48:51 +0100 > From: Vladimir 'φ-coder/phcoder' Serbinenko <phco...@gmail.com> > To: grub-devel@gnu.org > > Going through e-mail archives found this one, looks like it got > forgotten. Fixed problems with it and simplified greatly and committed. > On 18.12.2011 04:16, Peter Lustig wrote: > > On 17.12.2011 00:41, Vladimir 'phcoder' Serbinenko wrote: > > > >> /* nthibr.c - tests whether an MS Windows system partition is > >> hibernated */ > >> /* > >> * GRUB -- GRand Unified Bootloader > >> * Copyright (C) 2007,2008,2009,2011 Free Software Foundation, Inc. > >> * > >> * GRUB is free software: you can redistribute it and/or modify > >> * it under the terms of the GNU General Public License as > >> published by > >> * the Free Software Foundation, either version 3 of the License, or > >> * (at your option) any later version. > >> * > >> * GRUB is distributed in the hope that it will be useful, > >> * but WITHOUT ANY WARRANTY; without even the implied warranty of > >> * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >> * GNU General Public License for more details. > >> * > >> * You should have received a copy of the GNU General Public License > >> * along with GRUB. If not, see<http://www.gnu.org/licenses/>. > >> */ > >> > >> #include<grub/types.h> > >> #include<grub/mm.h> > >> #include<grub/disk.h> > >> #include<grub/file.h> > >> #include<grub/misc.h> > >> #include<grub/dl.h> > >> #include<grub/extcmd.h> > >> #include<grub/lib/arg.h> > >> #include<grub/err.h> > >> #include<grub/i18n.h> > >> > >> GRUB_MOD_LICENSE("GPLv3+"); > >> > >> /* Define this 'empty' array to let the '-h' and '-u' switches be > >> processed */ > >> static const struct grub_arg_option options[] = { > >> {0, 0, 0, 0, 0, 0} > >> }; > >> Please remove this and use grub_command_t > >> I understand now. What threw me off was seeing the 'hello' module use > >> 'extcmd.' > >>>> static grub_err_t > >>>> grub_cmd_nthibr (grub_extcmd_context_t ctxt __attribute__ ((unused)), > >>>> int argc, char **args) > >>>> { > >>>> char *partition_name = 0, *hibr_file_path = 0, hibr_file_magic[5]; > >>>> grub_size_t name_length; > >>>> grub_ssize_t magic_size; > >>>> grub_disk_t partition; > >>>> grub_file_t hibr_file = 0; > >>>> > >>>> /* Check argument count */ > >>>> if (!argc) > >>>> return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("too few > >>>> arguments")); > >>>> else if (argc> 1) > >>>> return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("too many > >>>> arguments")); > >>>> > >>> Either ignore trailing argument or put both in one check != 1 > >>>> /* Make copy of partition specifier, so it can be modified (if > >>>> needed) */ > >>>> name_length = grub_strlen (args[0]); > >>>> partition_name = grub_zalloc (name_length + 3); > >>>> if (!partition_name) > >>>> goto exit; > >>>> grub_strcpy (partition_name, args[0]); > >>>> > >>>> /* Ensure partition specifier start with a '(' */ > >>>> if (partition_name[0] != '(') > >>>> { > >>>> grub_memmove (partition_name + 1, partition_name, > >>>> name_length++); > >>>> partition_name[0] = '('; > >>>> } > >>>> > >>>> /* Ensure partition specifier ends with a ')' */ > >>>> if (partition_name[name_length - 1] != ')') > >>>> partition_name[(++name_length) - 1] = ')'; > >>>> > >>>> /* Check if partition actually exists */ > >>>> partition_name[name_length - 1] = '\0'; > >>>> partition = grub_disk_open (partition_name + 1); > >>>> if (!partition) > >>>> goto exit; > >>>> grub_disk_close (partition); > >>>> partition_name[name_length - 1] = ')'; > >>>> > >>>> /* Build path to 'hiberfil.sys' */ > >>>> hibr_file_path = grub_malloc ((grub_strlen (partition_name) > >>>> + grub_strlen ("/hiberfil.sys") + > >>>> 1)); > >>>> if (!hibr_file_path) > >>>> goto exit; > >>>> grub_strcpy (hibr_file_path, partition_name); > >>>> grub_strcat (hibr_file_path, "/hiberfil.sys"); > >>>> > >>> It would be more efficient if you just try to open file and see what > >>> error you get. Actually you don't even need to distinguish between > >>> error cases here since you return the error you encountered. This will > >>> also save one useless alloc. > >>>> /* Try to open 'hiberfil.sys' */ > >>>> hibr_file = grub_file_open (hibr_file_path); > >>>> if (!hibr_file) > >>>> { > >>>> if (grub_errno == GRUB_ERR_FILE_NOT_FOUND) > >>>> grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("'hiberfil.sys' not > >>>> found")); > >>> This overriding is unnecessary. FS code already provides this message > >> This was intentional: the default message ('file not found') is not > >> particularly helpful in this case since the > >> command's description nowhere mentions what file it's looking for (let > >> alone that any file is needed for it to > >> function). I've updated this section to pop the first error off the > >> stack to eliminate the duplication (and added > >> a descriptive comment). > >>>> goto exit; > >>>> } > >>>> > >>>> /* Try to read magic number of 'hiberfil.sys' */ > >>>> magic_size = sizeof (hibr_file_magic) - 1; > >>>> grub_memset (hibr_file_magic, 0, sizeof (hibr_file_magic)); > >>>> if (grub_file_read (hibr_file, hibr_file_magic, magic_size)< > >>>> magic_size) > >>>> { > >>>> if (!grub_errno) > >>>> grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("'hiberfil.sys' too > >>>> small")); > >>>> goto exit; > >>>> } > >>>> > >>>> /* Return SUCCESS if magic indicates file is active; else return > >>>> FAILURE */ > >>>> if (!grub_strncasecmp ("hibr", hibr_file_magic, magic_size)) > >>>> grub_error (GRUB_ERR_NONE, "true"); > >>>> else > >>>> grub_error (GRUB_ERR_TEST_FAILURE, "false"); > >>>> > >>>> exit: > >>>> /* Ensure unmanaged resources are cleaned up */ > >>>> if (partition_name) > >>>> grub_free (partition_name); > >>>> if (hibr_file_path) > >>>> grub_free (hibr_file_path); > >>> No need to check that argument to free is non-NULL. > >>>> if (hibr_file) > >>>> grub_file_close (hibr_file); > >>>> > >>>> return grub_errno; > >>>> } > >>>> > >>>> static grub_extcmd_t cmd; > >>>> > >>>> GRUB_MOD_INIT (nthibr) > >>>> { > >>>> cmd = grub_register_extcmd ("nthibr", grub_cmd_nthibr, 0, > >>>> N_("DEVICE"), > >>>> N_("Test whether an NT system > >>>> partition " > >>>> "is hibernated."), > >>>> options); > >>>> } > >>>> > >>>> GRUB_MOD_FINI (nthibr) > >>>> { > >>>> grub_unregister_extcmd (cmd); > >>>> } > >>>> > >> ~Peter Lustig > >> -------------- next part -------------- > >> An HTML attachment was scrubbed... > >> URL:< > http://lists.gnu.org/archive/html/grub-devel/attachments/20111217/71127c00/attachment.html > > > >> > >> -------------- next part -------------- > >> An embedded and charset-unspecified text was scrubbed... > >> Name: nthibr.c > >> URL:< > http://lists.gnu.org/archive/html/grub-devel/attachments/20111217/71127c00/attachment.c > > > >> > >> > >> ------------------------------ > >> > >> Message: 2 > >> Date: Sat, 17 Dec 2011 12:39:41 +0100 > >> From: Vladimir '?-coder/phcoder' Serbinenko <phco...@gmail.com> > >> To:grub-devel@gnu.org > >> Subject: Re: New command to check NT's hibernation state > >> Message-ID:<4eec7f7d.2050...@gmail.com> > >> Content-Type: text/plain; charset=UTF-8; format=flowed > >> > >> > >>>>> /* Try to open 'hiberfil.sys' */ > >>>>> hibr_file = grub_file_open (hibr_file_path); > >>>>> if (!hibr_file) > >>>>> { > >>>>> if (grub_errno == GRUB_ERR_FILE_NOT_FOUND) > >>>>> grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("'hiberfil.sys' not > >>>>> found")); > >>>> This overriding is unnecessary. FS code already provides this message > >>> This was intentional: the default message ('file not found') is not > >>> particularly helpful in this case since the > >>> command's description nowhere mentions what file it's looking for (let > >>> alone that any file is needed for it to > >>> function). I've updated this section to pop the first error off the > >>> stack to eliminate the duplication (and added > >>> a descriptive comment). > >> This means that your copy of GRUB is old. Newer ones do have the > >> descriptive message. And even if it wasn't the case it's something to be > >> fixed in FS code, not by replacing error message here. > > I've been using the latest release (1.99), so this fix must still be on > > the development branch. But I do appreciate > > the idea of fixing something once at the source instead of at multiple > > points of consumption. > > > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel > > > > > > > > >
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
