From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Rc7FD-0003M1-Dn for mharc-grub-devel@gnu.org; Sat, 17 Dec 2011 22:16:59 -0500 Received: from eggs.gnu.org ([140.186.70.92]:54009) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rc7F9-0003Jl-V3 for grub-devel@gnu.org; Sat, 17 Dec 2011 22:16:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rc7F8-0005cS-93 for grub-devel@gnu.org; Sat, 17 Dec 2011 22:16:55 -0500 Received: from mail-qw0-f48.google.com ([209.85.216.48]:33889) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rc7F8-0005cM-2w for grub-devel@gnu.org; Sat, 17 Dec 2011 22:16:54 -0500 Received: by qadc16 with SMTP id c16so2972762qad.0 for ; Sat, 17 Dec 2011 19:16:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type; bh=UjBygSUv2+T78JONDpwrtSrUv+h3jj2m1m0/T20cBvM=; b=WmoBPE7/PdbAXL0++V7OUiHsyXZVUA5NY98U07XE5H1UuZYcd4PgW/ZzKE/B85+2uu TM0SaISk7ZJcsve74zHjRvB1DxymtLAXpr4hoFgaUGwKEVFA208FxzKhHNoZmMKVUrJ3 e85UpbZQJQVtKUX5Kyq/weRvGrP7Yg4eG3MT8= Received: by 10.224.117.143 with SMTP id r15mr19292071qaq.36.1324178213627; Sat, 17 Dec 2011 19:16:53 -0800 (PST) Received: from [127.0.0.1] (pool-74-104-23-109.bstnma.east.verizon.net. [74.104.23.109]) by mx.google.com with ESMTPS id dj9sm30366720qab.18.2011.12.17.19.16.51 (version=SSLv3 cipher=OTHER); Sat, 17 Dec 2011 19:16:52 -0800 (PST) Message-ID: <4EED5B22.1090904@gmail.com> Date: Sat, 17 Dec 2011 22:16:50 -0500 From: Peter Lustig User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0) Gecko/20111105 Thunderbird/8.0 MIME-Version: 1.0 To: grub-devel@gnu.org Subject: Re: New command to check NT's hibernation state References: In-Reply-To: Content-Type: multipart/mixed; boundary="------------020401080108010803030300" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.216.48 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Dec 2011 03:16:57 -0000 This is a multi-part message in MIME format. --------------020401080108010803030300 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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. > */ > > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > > 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: > -------------- next part -------------- > An embedded and charset-unspecified text was scrubbed... > Name: nthibr.c > URL: > > ------------------------------ > > Message: 2 > Date: Sat, 17 Dec 2011 12:39:41 +0100 > From: Vladimir '?-coder/phcoder' Serbinenko > To:grub-devel@gnu.org > Subject: Re: New command to check NT's hibernation state > Message-ID:<4EEC7F7D.2050900@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. --------------020401080108010803030300 Content-Type: text/plain; name="nthibr.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="nthibr.c" /* 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 . */ #include #include #include #include #include #include #include #include GRUB_MOD_LICENSE("GPLv3+"); static grub_err_t grub_cmd_nthibr (grub_command_t cmd __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_file_t hibr_file = 0; /* Check argument count */ if (argc != 1) return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument count")); /* 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] = ')'; /* 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"); /* Try to open 'hiberfil.sys' */ hibr_file = grub_file_open (hibr_file_path); if (!hibr_file) 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 */ grub_free (partition_name); grub_free (hibr_file_path); if (hibr_file) grub_file_close (hibr_file); return grub_errno; } static grub_command_t cmd; GRUB_MOD_INIT (nthibr) { cmd = grub_register_command ("nthibr", grub_cmd_nthibr, N_("DEVICE"), N_("Test whether an NT system partition " "is hibernated.")); } GRUB_MOD_FINI (nthibr) { grub_unregister_command (cmd); } --------------020401080108010803030300--