From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Rbn1j-0001pe-Gk for mharc-grub-devel@gnu.org; Sat, 17 Dec 2011 00:41:43 -0500 Received: from eggs.gnu.org ([140.186.70.92]:39686) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rbn1g-0001pY-Ct for grub-devel@gnu.org; Sat, 17 Dec 2011 00:41:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rbn1e-00006P-BQ for grub-devel@gnu.org; Sat, 17 Dec 2011 00:41:40 -0500 Received: from mail-qy0-f169.google.com ([209.85.216.169]:45793) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rbn1e-00006I-4V for grub-devel@gnu.org; Sat, 17 Dec 2011 00:41:38 -0500 Received: by qcsd17 with SMTP id d17so2535698qcs.0 for ; Fri, 16 Dec 2011 21:41:37 -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=0FdwPsbzpBPOoEKFulChJ/TFzm0dCIu49ejFSpvL8zo=; b=pcc2TBv6ffdAND/pClaT3kGwiOO+3eebMeZfzkqqxN0nqFfc6IfIaKHztergt9Wz2N jLqr1iVk7xQ9F1xFfoJHzQNm/Xur7y1SWiackqRMlI52BQ3Nhkkgxh7+wzDJlN7oNLb+ e3Gnhd3EzwIBRiJdvMF9gr+iK+sU1GHalErNM= Received: by 10.229.76.78 with SMTP id b14mr2669166qck.138.1324100497147; Fri, 16 Dec 2011 21:41:37 -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 dk9sm24272061qab.0.2011.12.16.21.41.35 (version=SSLv3 cipher=OTHER); Fri, 16 Dec 2011 21:41:36 -0800 (PST) Message-ID: <4EEC2B8E.9060003@gmail.com> Date: Sat, 17 Dec 2011 00:41:34 -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="------------030205030907050606030504" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.216.169 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, 17 Dec 2011 05:41:42 -0000 This is a multi-part message in MIME format. --------------030205030907050606030504 Content-Type: multipart/alternative; boundary="------------070106080604050502000200" --------------070106080604050502000200 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 16.12.2011 10:30, 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 --------------070106080604050502000200 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 16.12.2011 10:30, 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
--------------070106080604050502000200-- --------------030205030907050606030504 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) { if (grub_errno == GRUB_ERR_FILE_NOT_FOUND) { /* make this message more descriptive */ grub_error_pop (); grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("'hiberfil.sys' not found")); } 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); } --------------030205030907050606030504--