From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1RWn2m-0005j7-Oq for mharc-grub-devel@gnu.org; Sat, 03 Dec 2011 05:42:08 -0500 Received: from eggs.gnu.org ([140.186.70.92]:49955) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RWn2k-0005ip-Ge for grub-devel@gnu.org; Sat, 03 Dec 2011 05:42:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RWn2j-0006U1-5i for grub-devel@gnu.org; Sat, 03 Dec 2011 05:42:06 -0500 Received: from mail-ww0-f49.google.com ([74.125.82.49]:54556) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RWn2i-0006Tx-QV for grub-devel@gnu.org; Sat, 03 Dec 2011 05:42:05 -0500 Received: by wgbdt11 with SMTP id dt11so2628379wgb.30 for ; Sat, 03 Dec 2011 02:42:04 -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:content-transfer-encoding; bh=Y9LiTN3lLRkoFOJiUJ1YRYeCl6460sLb9MwEY3MXcsI=; b=CPR2s5EfCUwysxbp3YCoJVQBabeS8jzmBeZdu4E6LvW6/CKZGL4HnnwNxTgYSzSksV rylntZRNWdWUtZF+jrX0y6tLFOv4rJKrXGAnzqgFagx8qvAAPrcJMzNZLaclqradRp7M vOU1lVKHGyc6AKSZwa4hUj6jDlM+23bZtR5X4= Received: by 10.216.132.215 with SMTP id o65mr489903wei.17.1322908922228; Sat, 03 Dec 2011 02:42:02 -0800 (PST) Received: from debian.x201.phnet (25-53.203-62.cust.bluewin.ch. [62.203.53.25]) by mx.google.com with ESMTPS id dw6sm5370383wib.12.2011.12.03.02.41.59 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 03 Dec 2011 02:42:00 -0800 (PST) Message-ID: <4ED9FCF6.2010703@gmail.com> Date: Sat, 03 Dec 2011 11:41:58 +0100 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.24) Gecko/20111114 Icedove/3.1.16 MIME-Version: 1.0 To: grub-devel@gnu.org Subject: Re: New command to check NT's hibernation state References: <4ED9AC2B.3080307@gmail.com> In-Reply-To: <4ED9AC2B.3080307@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 74.125.82.49 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: Sat, 03 Dec 2011 10:42:07 -0000 On 03.12.2011 05:57, Peter Lustig 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} > }; > No need for empty array. passing 0 instead of this array works. > static grub_err_t > grub_cmd_nthibr (grub_extcmd_context_t ctxt __attribute__ ((unused)), > int argc, char **args) > { > grub_err_t status = GRUB_ERR_NONE; > char *partition_name, *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) > { > status = grub_error (GRUB_ERR_BAD_ARGUMENT, > N_("too few arguments specified")); > goto exit; > } > else if (argc> 1) > { > status = grub_error (GRUB_ERR_BAD_ARGUMENT, > N_("too many arguments specified")); > goto exit; > } > > partition_name = args[0]; > name_length = grub_strlen (partition_name); > > /* Check if partition specifier 'looks right' */ > if (partition_name[0] != '(' || partition_name[name_length - 1] != ')') > { > status = grub_error (GRUB_ERR_BAD_FILENAME, > N_("invalid partition specifier")); > goto exit; > } > I would recommend adding '(' and ')' if none found rather than giving error. > /* Check if partition actually exists */ > partition_name[name_length - 1] = '\0'; > partition = grub_disk_open (partition_name + 1); > if (!partition) > { > status = grub_error (GRUB_ERR_UNKNOWN_DEVICE, > N_("partition not found")); No need to run grub_error here. grub_disk_open already sets grub_errno and grub_errmsg. Just status = grub_errno is enough. > goto exit; > } > else > { > grub_disk_close (partition); > partition_name[name_length - 1] = ')'; > } > No need for 'else' clause (goto already takes care of it) > /* Build path to 'hiberfil.sys' */ > hibr_file_path = grub_malloc (grub_strlen (partition_name) > + grub_strlen ("/hiberfil.sys") + 1); > if (!hibr_file_path) > { > status = grub_error (GRUB_ERR_OUT_OF_MEMORY, > N_("out of memory")); > goto exit; > } > else > { > grub_strcpy (hibr_file_path, partition_name); > grub_strcat (hibr_file_path, "/hiberfil.sys"); > } > Same here (no need for grub_error and else) > /* Try to open 'hiberfil.sys' */ > hibr_file = grub_file_open (hibr_file_path); > if (!hibr_file) > { > status = grub_error (GRUB_ERR_FILE_NOT_FOUND, > N_("'hiberfil.sys' not found")); ditto > 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) > { > status = grub_error (GRUB_ERR_BAD_FILE_TYPE, > N_("'hiberfil.sys' too small")); you need to conditionalize call to grub_error. if (!grub_errno) ... since incomplete read may indicate an FS problem in which case you want the original error message to remain. > goto exit; > } > > /* Return SUCCESS if magic indicates file is active; else return FAILURE */ > if (!grub_strncasecmp ("hibr", hibr_file_magic, magic_size)) > grub_puts (N_("The system is hibernated.")); > else > { > grub_puts (N_("The system is NOT hibernated.")); > status = GRUB_ERR_TEST_FAILURE; > } > We also do grub_error (GRUB_ERR_TEST_FAILURE, "false"); > exit: > /* Ensure unmanaged resources are cleaned up */ > if (hibr_file_path) > grub_free (hibr_file_path); grub_free already checks for not-NULL pointers. > if (hibr_file) > grub_file_close (hibr_file); > > return status; You can eliminate status and the need of keeping one by using return grub_errno; > } -- Regards Vladimir 'φ-coder/phcoder' Serbinenko