All of lore.kernel.org
 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 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.