grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Lustig <peter.lustig.7db@gmail.com>
To: grub-devel@gnu.org
Subject: Re: New command to check NT's hibernation state
Date: Sat, 10 Dec 2011 01:03:50 -0500	[thread overview]
Message-ID: <4EE2F646.1050309@gmail.com> (raw)
In-Reply-To: <mailman.207.1322931671.23776.grub-devel@gnu.org>

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

       reply	other threads:[~2011-12-10  6:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <mailman.207.1322931671.23776.grub-devel@gnu.org>
2011-12-10  6:03 ` Peter Lustig [this message]
2011-12-16 15:25   ` New command to check NT's hibernation state Vladimir 'φ-coder/phcoder' Serbinenko
2011-12-16 15:30   ` 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.213.1324054906.20674.grub-devel@gnu.org>
2011-12-17  5:41 ` Peter Lustig
2011-12-17 11:39   ` 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EE2F646.1050309@gmail.com \
    --to=peter.lustig.7db@gmail.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).