Re: About the CLI of both grub-mkrescue versions

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 01.10.2014 10:25, Thomas Schmitt wrote:
> Hi,
> 
> to substantiate my proposal of renaming young grub-mkrescue.c to
> grub-mkiso.c and to add a built-in emulation of grub-mkrescue(.in),
> here the necessary code which i tested standalone with valgrind.
> 
> The decision which parser to use would be made in main():
> 
I think that old parser is better. The only reason the change happened
is that it's a bug that sneaked in during migration to C. It should be
fixed.
> --------------------------------------------------------------
> 
>   char *cpt;
>   ...
>   /* Get leaf name of argv[0] */
>   for (cpt = argv[0] + strlen (argv[0]) - 1; cpt >= argv[0]; cpt--)
>     if (*cpt == '/')
>       break;
>   cpt++;
>   if (strcmp (cpt, "grub-mkrescue") == 0)
>     {
>       arg_parser_mkrescue (argc, argv);
>     }
>   else
>     {
>       argp_parse (&argp, argc, argv, 0, 0, 0);
>     }
> 
> --------------------------------------------------------------
> 
> The help text is derived from grub-mkrescue.in of GRUB 2.00.
> 
> --------------------------------------------------------------
> 
> static void
> printc (char *line)
> {
>   printf ("%s\n", line);
> }
> 
> static void
> print_mkrescue_help (char *prog_name)
> {
>   printf ("%s %s %s\n", _("Usage:"), prog_name, _("[OPTION] SOURCE..."));
>   printc (_("Make GRUB CD-ROM, disk, pendrive and floppy bootable image."));
>   printc (_("-h, --help"));
>   printc (_("        print this message and exit"));
>   printc (_("-v, --version"));
>   printc (_("        print the version information and exit"));
>   printc (_("-o, --output=FILE"));
>   printc (_("        save output in FILE [required]"));
>   printc (_("--rom-directory=DIR"));
>   printc (_("        save ROM images in DIR [optional]"));
>   printc (_("--xorriso=FILE"));
>   printc (_("        use FILE as xorriso [optional]"));
>   printc (_("Not supported any more are:"));
>   printc (_("        --modules , --grub-mkimage , --override-directory"));
>   printc (_("Other arguments get forwarded to xorriso -as mkisofs"));
>   printc (_("emulation."));
>   printc ("");
>   printf ("%s %s\n", prog_name, _("generates a bootable rescue image"));
>   printc (_("with specified source files, source directories, or mkisofs"));
>   printc (_("options listed by the output of `xorriso -as mkisofs -help'"));
>   printc ("");
>   printc (_("Note: Do not use option \"--\" unless you want to submit"));
>   printc (_("      native xorriso commands instead of file paths or"));
>   printc (_("      mkisofs options. See man xorrisofs and man xorriso."));
>   printc ("");
>   printc (_("Report bugs to <address@hidden>."));
>   printc (_("Mail xorriso support requests to <address@hidden>."));
>   printc ("");
>   printc (_("This is program grub-mkiso emulating the option"));
>   printc (_("interpretation of legacy program grub-mkrescue."));
>   printc (_("grub-mkiso in its native mode has more advanced options."));
>   printc (_("But that mode demands to separate grub-mkiso options"));
>   printc (_("and xorriso options by a double dash \"--\", which xorriso"));
>   printc (_("will not get to see."));
> }
> 
> --------------------------------------------------------------
> 
> The parser function implements the promised options and collects
> the xorriso -as mkisofs options into the same char pointer array
> as does the existing parser in the C program.
> In particular it sets the values of these variables:
> 
> static char *rom_directory;
> static int xorriso_tail_argc;
> static int xorriso_tail_arg_alloc;
> static char **xorriso_tail_argv;
> static char *output_image;
> static char *xorriso;
> 
> --------------------------------------------------------------
> 
> static void
> arg_parser_mkrescue (int argc, char *argv[])
> {
>   int i;
> 
>   for (i = 1; i < argc; i++)
>     {
>       if (strcmp (argv[i], "-h") == 0 || strcmp (argv[i], "--help") == 0)
>         {
>           print_mkrescue_help (argv[0]);
>           exit (0);
>         }
>       else if (strcmp (argv[i], "-v") == 0
>                || strcmp (argv[i], "--version") == 0)
>         {
>           printf ("%s %s %s\n", argv[0], PACKAGE_NAME, PACKAGE_VERSION);
>           exit (0);
>         }
>       else if (strcmp (argv[i], "--modules") == 0)
>         {
>           grub_util_error (_("Option --modules is not supported any more"));
>         }
>       else if (strncmp (argv[i], "--modules=", 10) == 0)
>         {
>           grub_util_error (_("Option --modules= is not supported any more"));
>         }
>       else if (strcmp (argv[i], "-o") == 0
>                || strcmp (argv[i], "--output") == 0)
>         {
>           if (i == argc - 1)
>             grub_util_error (_("option requires an argument -- `%s'"),
>                              argv[i]);
>           i++;
>           free (output_image);
>           output_image = xstrdup (argv[i]);
>         }
>       else if (strncmp (argv[i], "--output=", 9) == 0)
>         {
>           free (output_image);
>           output_image = xstrdup (argv[i] + 9);
>         }
>       else if (strcmp (argv[i], "--rom-directory") == 0)
>         {
>           if (i == argc - 1)
>             grub_util_error (_("option requires an argument -- `%s'"),
>                              argv[i]);
>           i++;
>           free (rom_directory);
>           rom_directory = xstrdup (argv[i]);
>         }
>       else if (strncmp (argv[i], "--rom-directory=", 16) == 0)
>         {
>           free (rom_directory);
>           rom_directory = xstrdup (argv[i] + 16);
>         }
>       else if (strcmp (argv[i], "--override-directory") == 0)
>         {
>           grub_util_error(
>                    _("Option --override-directory is not supported any 
> more"));
>         }
>       else if (strncmp (argv[i], "--override-directory=", 21) == 0)
>         {
>           grub_util_error(
>                   _("Option --override-directory= is not supported any 
> more"));
>         }
>       else if (strcmp (argv[i], "--xorriso") == 0)
>         {
>           if (i == argc - 1)
>             grub_util_error (_("option requires an argument -- `%s'"),
>                              argv[i]);
>           i++;
>           free (xorriso);
>           xorriso = xstrdup (argv[i]);
>         }
>       else if (strncmp (argv[i], "--xorriso=", 10) == 0)
>         {
>           free (xorriso);
>           xorriso = xstrdup (argv[i] + 10);
>         }
>       else
>         {
>           if (xorriso_tail_arg_alloc <= xorriso_tail_argc)
>             {
>               xorriso_tail_arg_alloc = 2 * (4 + xorriso_tail_argc);
>               xorriso_tail_argv = xrealloc (xorriso_tail_argv,
>                                       sizeof (xorriso_tail_argv[0])
>                                       * xorriso_tail_arg_alloc);
>             }
>           xorriso_tail_argv[xorriso_tail_argc++] = xstrdup (argv[i]);
>         }
>     }
> }
> 
> --------------------------------------------------------------
> 
> There remains the incompatibility that i could not find
> equivalents of the following grub-mkrescue.in features:
> 
>   --modules=MODULES
>           pre-load specified modules MODULES
>   --grub-mkimage=FILE
>           use FILE as grub-mkimage
>   --override-directory
>           "Intentionally undocumented"
> 
> --------------------------------------------------------------
> 
> The valgrind test was made with a mock-up of grub-mkiso.c
> consisting of some copied util/grub-*.c functions and a main()
> which prints the content of the variables after parsing.
> 
>   ln -s grub-mkiso grub-mkrescue
>   valgrind ./grub-mkrescue -o output.iso -J --rom-directory=./ROMDIR -P 
> YET_ANOTHER_OS.ORG --md5 --emul-toc ./my_os_payload 
> --xorriso="$HOME"/xorriso-1.3.8/xorriso/xorriso
> 
> yielded:
> 
>   output_image=           'output.iso'
>   rom_directory=          './ROMDIR'
>   xorriso=                '/home/thomas/xorriso-1.3.8/xorriso/xorriso'
>   xorriso_tail_arg_alloc= 8
>   xorriso_tail_argc=      6
>   xorriso_tail_argv[ 0]= '-J'
>   xorriso_tail_argv[ 1]= '-P'
>   xorriso_tail_argv[ 2]= 'YET_ANOTHER_OS.ORG'
>   xorriso_tail_argv[ 3]= '--md5'
>   xorriso_tail_argv[ 4]= '--emul-toc'
>   xorriso_tail_argv[ 5]= './my_os_payload'
>   ==9810== 
>   ==9810== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 1)
>   ...
>   ==9810== LEAK SUMMARY:
>   ==9810==    definitely lost: 0 bytes in 0 blocks.
>   ==9810==      possibly lost: 0 bytes in 0 blocks.
>   ==9810==    still reachable: 185 bytes in 10 blocks.
>   ==9810==         suppressed: 0 bytes in 0 blocks.
> 
> The memory leaks are caused by allocated storage of global
> variables.
> 
> 
> Have a nice day :)
> 
> Thomas
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

signature.asc
Description: OpenPGP digital signature