Re: New command to check NT's hibernation state

[Vladimir 'φ-coder/phcoder' Serbinenko]

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     <address@hidden>
>> To:address@hidden
>> Subject: Re: New command to check NT's hibernation state
>> Message-ID:<address@hidden>
>> 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
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

signature.asc
Description: OpenPGP digital signature