All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: New command to check NT's hibernation state
       [not found] <mailman.213.1324054906.20674.grub-devel@gnu.org>
@ 2011-12-17  5:41 ` Peter Lustig
  2011-12-17 11:39   ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Lustig @ 2011-12-17  5:41 UTC (permalink / raw)
  To: grub-devel


[-- Attachment #1.1: Type: text/plain, Size: 5899 bytes --]

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

[-- Attachment #1.2: Type: text/html, Size: 12574 bytes --]

[-- Attachment #2: nthibr.c --]
[-- Type: text/plain, Size: 3842 bytes --]

/* 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/file.h>
#include <grub/misc.h>
#include <grub/dl.h>
#include <grub/command.h>
#include <grub/err.h>
#include <grub/i18n.h>

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;
}
\f
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);
}

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-17  5:41 ` New command to check NT's hibernation state Peter Lustig
@ 2011-12-17 11:39   ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-12-17 11:39 UTC (permalink / raw)
  To: grub-devel


>>>    /* 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.

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2013-11-04 13:13     ` Andrey Borzenkov
@ 2013-11-04 13:24       ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-11-04 13:24 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

On 04.11.2013 14:13, Andrey Borzenkov wrote:
> В Mon, 04 Nov 2013 01:48:51 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> Going through e-mail archives found this one, looks like it got
>> forgotten. Fixed problems with it and simplified greatly and committed.
> 
> Is command name a typo?
> 
>   cmd = grub_register_command ("check_nt_hibrerfil", grub_cmd_nthibr,
> 
> Looks like one "r" too much.
> 
You're right
> Also any reason to omit final "e"? check_nt_hiberfile (or even
> check_nt_hiber_file) looks more readable.
> 
To be in line with the file name: hiberfil.sys (which itself follows 8.3)
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2013-11-04  0:48   ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-11-04 13:13     ` Andrey Borzenkov
  2013-11-04 13:24       ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 20+ messages in thread
From: Andrey Borzenkov @ 2013-11-04 13:13 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

В Mon, 04 Nov 2013 01:48:51 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> Going through e-mail archives found this one, looks like it got
> forgotten. Fixed problems with it and simplified greatly and committed.

Is command name a typo?

  cmd = grub_register_command ("check_nt_hibrerfil", grub_cmd_nthibr,

Looks like one "r" too much.

Also any reason to omit final "e"? check_nt_hiberfile (or even
check_nt_hiber_file) looks more readable.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
@ 2013-11-04  1:55 Peter Lustig
  0 siblings, 0 replies; 20+ messages in thread
From: Peter Lustig @ 2013-11-04  1:55 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 9653 bytes --]

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 <
phcoder@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 <phcoder@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     <phcoder@gmail.com>
> >> 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.
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
>
>
>
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 13993 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-18  3:16 ` Peter Lustig
  2011-12-22 12:10   ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-11-04  0:48   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-11-04 13:13     ` Andrey Borzenkov
  1 sibling, 1 reply; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-11-04  0:48 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 8763 bytes --]

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     <phcoder@gmail.com>
>> 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.
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-23  4:32 ` Peter Lustig
  2012-08-12  4:23   ` Peter Lustig
@ 2013-01-28 18:11   ` Vladimir 'φ-coder/phcoder' Serbinenko
  1 sibling, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-01-28 18:11 UTC (permalink / raw)
  To: The development of GNU GRUB, Peter Lustig

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

On 23.12.2011 05:32, Peter Lustig wrote:

> On 12/22/2011 13:10, Vladimir 'phcoder' Serbinenko wrote:
>> On 18.12.2011 04:16, Peter Lustig wrote:
>>> >      /* Return SUCCESS if magic indicates file is active; else
>>> return FAILURE */
>>> >      if (!grub_strncasecmp ("hibr", hibr_file_magic, magic_size))
>> What's the reason to use strncasecmp? Does the case changes? Usually
>> memcmp is the right way to check the signature. This also avoids the
>> need of memset and trailing zero byte.
> It can be either {'h', 'i', 'b', 'r'} (for Windows XP) or {'H', 'I',
> 'B', 'R'} (for Windows Vista/7).  Technically I should only be checking
> for these two values, but it is unlikely that the magic would have mixed
> case.  The only other values I know (from
> <http://www.msuiche.net/pres/PacSec07-slides-0.4.pdf>) that it can
> assume are {'w', 'a', 'k', 'e'}, {'l', 'i', 'n', 'k'}, and {'\0', '\0',
> '\0', '\0'}.  Using strncasecmp() seemed like a simple way to approach
> the problem.
> 



strncasecmp isn't for comparing te binary strings. In GRUB it's fairly
simplistic but this is conceptually wrong. Conceptually híbr and HIBR
would match even if GRUB currently doesn't do this kind of handling.

> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 





-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-23  4:32 ` Peter Lustig
@ 2012-08-12  4:23   ` Peter Lustig
  2013-01-28 18:11   ` Vladimir 'φ-coder/phcoder' Serbinenko
  1 sibling, 0 replies; 20+ messages in thread
From: Peter Lustig @ 2012-08-12  4:23 UTC (permalink / raw)
  To: grub-devel

On 12/22/2011 11:32 PM, Peter Lustig wrote:
> On 12/22/2011 13:10, Vladimir 'phcoder' Serbinenko wrote:
>> On 18.12.2011 04:16, Peter Lustig wrote:
>>> >      /* Return SUCCESS if magic indicates file is active; else 
>>> return FAILURE */
>>> >      if (!grub_strncasecmp ("hibr", hibr_file_magic, magic_size))
>> What's the reason to use strncasecmp? Does the case changes? Usually
>> memcmp is the right way to check the signature. This also avoids the
>> need of memset and trailing zero byte.
> It can be either {'h', 'i', 'b', 'r'} (for Windows XP) or {'H', 'I', 
> 'B', 'R'} (for Windows Vista/7).  Technically I should only be 
> checking for these two values, but it is unlikely that the magic would 
> have mixed case.  The only other values I know (from 
> <http://www.msuiche.net/pres/PacSec07-slides-0.4.pdf>) that it can 
> assume are {'w', 'a', 'k', 'e'}, {'l', 'i', 'n', 'k'}, and {'\0', 
> '\0', '\0', '\0'}.  Using strncasecmp() seemed like a simple way to 
> approach the problem.
Sorry to bump this thread.  Do you know if there are any plans to 
include the 'nthibr' module in a future release of GRUB.  I looked and 
didn't see any references to it in the Bazaar trunk.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
       [not found] <mailman.235.1324573308.26580.grub-devel@gnu.org>
@ 2011-12-23  4:32 ` Peter Lustig
  2012-08-12  4:23   ` Peter Lustig
  2013-01-28 18:11   ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 2 replies; 20+ messages in thread
From: Peter Lustig @ 2011-12-23  4:32 UTC (permalink / raw)
  To: grub-devel

On 12/22/2011 13:10, Vladimir 'phcoder' Serbinenko wrote:
> On 18.12.2011 04:16, Peter Lustig wrote:
>> >      /* Return SUCCESS if magic indicates file is active; else return FAILURE */
>> >      if (!grub_strncasecmp ("hibr", hibr_file_magic, magic_size))
> What's the reason to use strncasecmp? Does the case changes? Usually
> memcmp is the right way to check the signature. This also avoids the
> need of memset and trailing zero byte.
It can be either {'h', 'i', 'b', 'r'} (for Windows XP) or {'H', 'I', 
'B', 'R'} (for Windows Vista/7).  Technically I should only be checking 
for these two values, but it is unlikely that the magic would have mixed 
case.  The only other values I know (from 
<http://www.msuiche.net/pres/PacSec07-slides-0.4.pdf>) that it can 
assume are {'w', 'a', 'k', 'e'}, {'l', 'i', 'n', 'k'}, and {'\0', '\0', 
'\0', '\0'}.  Using strncasecmp() seemed like a simple way to approach 
the problem.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-18  3:16 ` Peter Lustig
@ 2011-12-22 12:10   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-11-04  0:48   ` Vladimir 'φ-coder/phcoder' Serbinenko
  1 sibling, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-12-22 12:10 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Peter Lustig

On 18.12.2011 04:16, Peter Lustig wrote:
>    /* Return SUCCESS if magic indicates file is active; else return FAILURE */
>    if (!grub_strncasecmp ("hibr", hibr_file_magic, magic_size))
What's the reason to use strncasecmp? Does the case changes? Usually 
memcmp is the right way to check the signature. This also avoids the 
need of memset and trailing zero byte.

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
       [not found] <mailman.171.1324141254.27913.grub-devel@gnu.org>
@ 2011-12-18  3:16 ` Peter Lustig
  2011-12-22 12:10   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-11-04  0:48   ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 2 replies; 20+ messages in thread
From: Peter Lustig @ 2011-12-18  3:16 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 7957 bytes --]

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 	<phcoder@gmail.com>
> 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.

[-- Attachment #2: nthibr.c --]
[-- Type: text/plain, Size: 3599 bytes --]

/* 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/file.h>
#include <grub/misc.h>
#include <grub/dl.h>
#include <grub/command.h>
#include <grub/err.h>
#include <grub/i18n.h>

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;
}
\f
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);
}

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-10  6:03 ` Peter Lustig
  2011-12-16 15:25   ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2011-12-16 15:30   ` Vladimir 'φ-coder/phcoder' Serbinenko
  1 sibling, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-12-16 15:30 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Peter Lustig

On 10.12.2011 07:03, 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<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
> 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
>        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);
> }
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-10  6:03 ` Peter Lustig
@ 2011-12-16 15:25   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2011-12-16 15:30   ` Vladimir 'φ-coder/phcoder' Serbinenko
  1 sibling, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-12-16 15:25 UTC (permalink / raw)
  To: grub-devel

On 10.12.2011 07:03, Peter Lustig wrote:
> I've followed all your recommendations (as far as possible).  Here's 
> the code again.
The empty options are still there. Your arguments for having it aren't 
command-specific so it's a wrong to put such workarounds here. Also 
please user command and not extcmd since you have no arguments.

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
       [not found] <mailman.207.1322931671.23776.grub-devel@gnu.org>
@ 2011-12-10  6:03 ` Peter Lustig
  2011-12-16 15:25   ` Vladimir 'φ-coder/phcoder' Serbinenko
  2011-12-16 15:30   ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 2 replies; 20+ messages in thread
From: Peter Lustig @ 2011-12-10  6:03 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 6072 bytes --]

On 12/3/2011 11:41 AM, Vladimir 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}
>> };
>>
> No need for empty array. passing 0 instead of this array works.
I have tried it both ways.  For some reason, passing the empty array 
works, but passing a null pointer doesn't.
That is, when I try the latter, the options '-u' and '-h' are treated as 
regular arguments to the command.
>> 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;
I was unaware 'grub_errno' was used so extensively.  Thanks for the tip: 
it definitely makes the logic cleaner.
>> }
>

I've followed all your recommendations (as far as possible).  Here's the 
code again.

~Peter Lustig

[-- Attachment #2: nthibr.c --]
[-- Type: text/plain, Size: 4387 bytes --]

/* 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}
};

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"));

  /* 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");

  /* 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"));
      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);
  if (hibr_file)
    grub_file_close (hibr_file);

  return grub_errno;
}
\f
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);
}

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-03  4:57 Peter Lustig
@ 2011-12-03 10:41 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-12-03 10:41 UTC (permalink / raw)
  To: grub-devel

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<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}
> };
>
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



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
@ 2011-12-03  4:57 Peter Lustig
  2011-12-03 10:41 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Lustig @ 2011-12-03  4:57 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 1798 bytes --]

On 02.12.2011 07:05, Vladimir Serbinenko wrote:

>> P.S.  Do you know if there are any plans in the future to implement
>> write operations for filesystems?  (I noticed raw disk writing is
>> currently supported.)  If not, is the reason due to the technical
>> challenges (e.g. NTFS only had read-only access in GNU/Linux for quite
>> a while) or just a general lack of interest?
> Neither. Reliability concerns. Writing to filesystem is potentially
> dangerous and needs to be throughly tested. Since writing is rare for
> bootloader we won't receive enough testing of these code pathes so every
> write will remain a risk. We can't have such risks
I see. Just curious.

>>> Thanks for this. However the coding style isn't according to our
>>> standards. Could you adjust to the standard? I recommend looking at
>>> other code and reading GNU Coding Standard. E.g. we don't use {0}
>>> initialiser, return value of commands, C++ comments or M$-$tyle
>>> variables.
>> Sorry about that.  I've reviewed the standard and perused the source
>> tree, as you suggested, to determine the best style.  Attached is my
>> updated file.
>>
> Much better. There are still few problems like the arbitrary limit (we
> avoid any arbitrary limits, you have one easy to avoid on filename
> length). Also we don't use such ABORT macros as they offer no
> readability advantage. Review in more details when time permits.
Argh. I remember reading about avoiding arbitrary limits too.

I would contend that in this case the macros do improve readability (each
reducing 4-5 lines of code down to 1-2 lines); yet, to be consistent with the
rest of the source base, I've removed them.

Here's the file again.  Maybe the third time's the charm?

Thanks for being a stickler for conformance ( I mean it :) ),
Peter Lustig


[-- Attachment #2: nthibr.c --]
[-- Type: text/plain, Size: 4609 bytes --]

/* 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}
};

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;
    }

  /* 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"));
      goto exit;
    }
  else 
    {
      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)
    {
      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");
    }

  /* 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"));
      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"));
      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;
    }

exit: 
  /* Ensure unmanaged resources are cleaned up */
  if (hibr_file_path)
    grub_free (hibr_file_path);
  if (hibr_file)
    grub_file_close (hibr_file);

  return status;
}
\f
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);
}

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-12-02  5:16 Peter Lustig
@ 2011-12-02  6:05 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-12-02  6:05 UTC (permalink / raw)
  To: The development of GNU GRUB

Hello.
On 02.12.2011 06:16, Peter Lustig wrote:
>> Thanks for this. However the coding style isn't according to our 
>> standards. Could you adjust to the standard? I recommend looking at 
>> other code and reading GNU Coding Standard. E.g. we don't use {0} 
>> initialiser, return value of commands, C++ comments or M$-$tyle 
>> variables. 
>
> Sorry about that.  I've reviewed the standard and perused the source 
> tree, as you suggested, to determine the best style.  Attached is my 
> updated file.
>
Much better. There are still few problems like the arbitrary limit (we 
avoid any arbitrary limits, you have one easy to avoid on filename 
length). Also we don't use such ABORT macros as they offer no 
readability advantage. Review in more details when time permits.
> Glad to be of help,
> Peter Lustig
>
> P.S.  Do you know if there are any plans in the future to implement 
> write operations for filesystems?  (I noticed raw disk writing is 
> currently supported.)  If not, is the reason due to the technical 
> challenges (e.g. NTFS only had read-only access in GNU/Linux for quite 
> a while) or just a general lack of interest?
Neither. Reliability concerns. Writing to filesystem is potentially 
dangerous and needs to be throughly tested. Since writing is rare for 
bootloader we won't receive enough testing of these code pathes so every 
write will remain a risk. We can't have such risks.
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-11-28  1:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2011-11-28 13:27   ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-11-28 13:27 UTC (permalink / raw)
  To: grub-devel

On 28.11.2011 02:03, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> M$-$tyle variables. 
Jordan Uggla pointed out that this was inappropriate to bash Microsoft. 
I'd like to apologize for it. I was very tired and just wanted to show 
the appreciation of the patch and point out what needs to be fixed.

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New command to check NT's hibernation state
  2011-11-27 23:49 Peter Lustig
@ 2011-11-28  1:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2011-11-28 13:27   ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 20+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2011-11-28  1:03 UTC (permalink / raw)
  To: grub-devel

On 28.11.2011 00:49, Peter Lustig wrote:
> Hello all,
>
Hello
>   I would like to contribute a small module I wrote to read the header 
> of MS Window's "hiberfil.sys" and determine whether it is in use (i.e. 
> Windows has been suspended to disk).  What is the procedure for 
> donating code?  Would I need commit rights to the repository, or would 
> someone else do it for me?  The source for the module is attached.
>
Thanks for this. However the coding style isn't according to our 
standards. Could you adjust to the standard? I recommend looking at 
other code and reading GNU Coding Standard. E.g. we don't use {0} 
initialiser, return value of commands, C++ comments or M$-$tyle variables.
> Thanks,
> Peter Lustig
>
> P.S. I have tested the code and verified it works for Windows 7.  
> Also, from what I understand about "hiberfil.sys" from the SandMan 
> project (cf. <http://www.msuiche.net/pres/PacSec07-slides-0.4.pdf>), 
> it should work for Windows Vista and XP as well.
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



^ permalink raw reply	[flat|nested] 20+ messages in thread

* New command to check NT's hibernation state
@ 2011-11-27 23:49 Peter Lustig
  2011-11-28  1:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 20+ messages in thread
From: Peter Lustig @ 2011-11-27 23:49 UTC (permalink / raw)
  To: grub-devel

[-- Attachment #1: Type: text/plain, Size: 638 bytes --]

Hello all,

   I would like to contribute a small module I wrote to read the header 
of MS Window's "hiberfil.sys" and determine whether it is in use (i.e. 
Windows has been suspended to disk).  What is the procedure for donating 
code?  Would I need commit rights to the repository, or would someone 
else do it for me?  The source for the module is attached.

Thanks,
Peter Lustig

P.S. I have tested the code and verified it works for Windows 7.  Also, 
from what I understand about "hiberfil.sys" from the SandMan project 
(cf. <http://www.msuiche.net/pres/PacSec07-slides-0.4.pdf>), it should 
work for Windows Vista and XP as well.

[-- Attachment #2: nthibr.c --]
[-- Type: text/plain, Size: 5809 bytes --]

/* 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/>.
 */

//======== INCLUDES ==========================================================//

//Common Utilities
#include <grub/mm.h>       //NULL
#include <grub/types.h>    //grub_*size_t
#include <grub/disk.h>     //grub_disk_t, grub_disk_*()
#include <grub/file.h>     //grub_file_t, grub_file_*()
#include <grub/misc.h>     //grub_str*(), grub_*puts(), grub_*printf()

//Module Management
#include <grub/dl.h>       //GRUB_MOD_*()
#include <grub/extcmd.h>   //grub_extcmd_*t, grub_*register_extcmd()
#include <grub/lib/arg.h>  //grub_arg_*, ARG_TYPE_*
#include <grub/err.h>      //grub_err_t, GRUB_ERR_*, grub_error()

//Internationalization
#include <grub/i18n.h>     //N_()

//======== PRAGMAS ===========================================================//

//permit use of the zero-initializer '{0}' for structures
#pragma GCC diagnostic ignored "-Wmissing-field-initializers"

//======== TYPE DEFINITIONS ==================================================//

typedef struct grub_arg_option grub_arg_option;

//======== MODULE VARIABLES ==================================================//

GRUB_MOD_LICENSE("GPLv3+");

static grub_extcmd_t cmd;

static const grub_arg_option options[] = {
  {
    .longarg  = "exit-codes", 
    .shortarg = 'x',
    .flags    = 0x0, 
    .doc      = N_("Display return codes used by this command and exit."), 
    .arg      = NULL,
    .type     = ARG_TYPE_NONE
  },
  {0} //end of list
};

//======== FUNCTIONS =========================================================//

static grub_err_t grub_cmd_nthibr (
  grub_extcmd_context_t ctxt, int argc, char* args[]
){
  grub_err_t status = GRUB_ERR_NONE;

  char *szPartName, szFilePath[32], szFileMagic[5] = {'\0'};

  grub_ssize_t length;

  grub_disk_t hPart;
  grub_file_t hFile = NULL;


  #define ABORT(_code, _message) {             \
    status = grub_error((_code), (_message));  \
    goto exit;                                 \
  }


  //If requested, print return codes and exit; else, proceed
  if (ctxt->state[0].set) {
    grub_printf(
      N_(
        "CODE  MEANING\n"
        "----  -------\n"
        "  %2d  system hibernated\n"
        "  %2d  system not hibernated\n"
        "  %2d  hibernation file too small\n"
        "  %2d  hibernation file not found\n"
        "  %2d  partition not found\n"
        "  %2d  invalid partition specifier\n"
        "  %2d  too few/many arguments provided\n"
      ),
      GRUB_ERR_NONE, GRUB_ERR_TEST_FAILURE, GRUB_ERR_BAD_FILE_TYPE,
      GRUB_ERR_FILE_NOT_FOUND, GRUB_ERR_BAD_FILENAME, GRUB_ERR_UNKNOWN_DEVICE, 
      GRUB_ERR_BAD_ARGUMENT
    );
    goto exit;
  }

  //Check argument count
  if (!argc) {
    ABORT( GRUB_ERR_BAD_ARGUMENT, N_("too few arguments specified") );
  } else if (argc > 1) {
    ABORT( GRUB_ERR_BAD_ARGUMENT, N_("too many arguments specified") );
  }

  szPartName = args[0];
  length = (grub_ssize_t) grub_strlen(szPartName);

  //Check if partition specifier 'looks right'
  if (szPartName[0] != '(' || szPartName[length-1] != ')')
    ABORT( GRUB_ERR_BAD_FILENAME, N_("invalid partition specifier") );

  //Check if partition actually exists
  szPartName[length-1] = '\0';
  if (NULL == (hPart = grub_disk_open(szPartName+1))) {
    ABORT( GRUB_ERR_UNKNOWN_DEVICE, N_("partition not found") );
  } else {
    grub_disk_close(hPart);
    szPartName[length-1] = ')';
  }

  //Build path to 'hiberfil.sys'
  grub_strncpy(szFilePath, szPartName, sizeof(szFilePath) - 1);
  grub_strncat ( 
    szFilePath, 
    "/hiberfil.sys", 
    sizeof(szFilePath) - grub_strlen(szFilePath) - 1
  );

  //Try to open 'hiberfil.sys'
  if (NULL == (hFile = grub_file_open(szFilePath))) {
    ABORT( GRUB_ERR_FILE_NOT_FOUND, N_("'hiberfil.sys' not found") );
  }

  //Try to read magic number of 'hiberfil.sys'
  length = (grub_ssize_t) (sizeof(szFileMagic) - 1);
  if (grub_file_read(hFile, szFileMagic, length) < length)
    ABORT( GRUB_ERR_BAD_FILE_TYPE, N_("'hiberfil.sys' too small") );

  //Return SUCCESS if magic indicates file is active; else return FAILURE
  if (0 == grub_strncasecmp("hibr", szFileMagic, length)) {
    grub_puts(N_("The system is hibernated."));
  } else {
    grub_puts(N_("The system is NOT hibernated."));
    status = GRUB_ERR_TEST_FAILURE;
  }

exit: 
  //Ensure 'hiberfil.sys' is closed
  if (hFile)
    grub_file_close(hFile);

  #undef ABORT
  return status;
} //grub_cmd_nthibr()


GRUB_MOD_INIT(nthibr) {
  cmd = grub_register_extcmd (
    "nthibr",                                                  //name
    grub_cmd_nthibr,                                           //function
    0x0,                                                       //flags = NONE
    N_("DEVICE"),                                              //summary = NONE
    N_("Test whether an NT system partition is hibernated."),  //description
    options                                                    //switches
  );
} //GRUB_MOD_INIT()


GRUB_MOD_FINI(nthibr) {
  grub_unregister_extcmd(cmd);
} //GRUB_MOD_FINI()

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2013-11-04 13:24 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <mailman.213.1324054906.20674.grub-devel@gnu.org>
2011-12-17  5:41 ` New command to check NT's hibernation state Peter Lustig
2011-12-17 11:39   ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-04  1:55 Peter Lustig
     [not found] <mailman.235.1324573308.26580.grub-devel@gnu.org>
2011-12-23  4:32 ` Peter Lustig
2012-08-12  4:23   ` Peter Lustig
2013-01-28 18:11   ` Vladimir 'φ-coder/phcoder' Serbinenko
     [not found] <mailman.171.1324141254.27913.grub-devel@gnu.org>
2011-12-18  3:16 ` Peter Lustig
2011-12-22 12:10   ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-04  0:48   ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-04 13:13     ` Andrey Borzenkov
2013-11-04 13:24       ` Vladimir 'φ-coder/phcoder' Serbinenko
     [not found] <mailman.207.1322931671.23776.grub-devel@gnu.org>
2011-12-10  6:03 ` Peter Lustig
2011-12-16 15:25   ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-12-16 15:30   ` Vladimir 'φ-coder/phcoder' Serbinenko
  -- strict thread matches above, loose matches on Subject: below --
2011-12-03  4:57 Peter Lustig
2011-12-03 10:41 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-12-02  5:16 Peter Lustig
2011-12-02  6:05 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-11-27 23:49 Peter Lustig
2011-11-28  1:03 ` Vladimir 'φ-coder/phcoder' Serbinenko
2011-11-28 13:27   ` Vladimir 'φ-coder/phcoder' Serbinenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.