Re: [PATCH] Add testpci command (v4)

[Andrey Borzenkov]

Does not "testpci" without parameters always return TRUE?

On Fri, Jan 25, 2013 at 9:12 AM, Jonathan McDowell <address@hidden> wrote:
> On Sun, Jan 20, 2013 at 11:27:28PM +0100, Vladimir 'φ-coder/phcoder'
> Serbinenko wrote:
>> On 13.10.2012 00:18, Jonathan McDowell wrote:
> (some code)
>> ARG_TYPE_INT means that the argument is decimal integer but it's not the
>> case here.
>
>> Please follow indent style.
>
>> You need to check grub_errno to catch any error in number parsing.
>> Also you need to update a little bit because of recent callback changes.
>> Other than this the patch is fine.
>
> Ok, I've updated against latest bzr and made the changes as above.
> Here's v4.
>
> Signed-off-by: Jonathan McDowell <address@hidden>
>
> -----
> === modified file 'docs/grub.texi'
> --- docs/grub.texi      2013-01-21 16:53:41 +0000
> +++ docs/grub.texi      2013-01-23 01:35:32 +0000
> @@ -3366,6 +3366,7 @@
>  * sendkey::                     Emulate keystrokes
>  * set::                         Set an environment variable
>  * source::                      Read a configuration file in same context
> +* testpci::                     Test for the existence of a PCI device
>  * true::                        Do nothing, successfully
>  * unset::                       Unset an environment variable
>  * uppermem::                    Set the upper memory size
> @@ -4147,6 +4148,18 @@
>  @end deffn
>
>
> address@hidden testpci
> address@hidden testpci
> +
> address@hidden Command testpci address@hidden @
> +  address@hidden address@hidden VENDORID}] @
> +  address@hidden PRODUCTID}]
> +Test for the existence of a given PCI device. Returns true if a device
> +matching the specified vendor/product and/or subvendor/subproduct information
> +is found.
> address@hidden deffn
> +
> +
>  @node true
>  @subsection true
>
>
> === modified file 'grub-core/Makefile.core.def'
> --- grub-core/Makefile.core.def 2013-01-16 19:44:11 +0000
> +++ grub-core/Makefile.core.def 2013-01-23 01:34:44 +0000
> @@ -862,6 +862,12 @@
>  };
>
>  module = {
> +  name = testpci;
> +  common = commands/testpci.c;
> +  enable = pci;
> +};
> +
> +module = {
>    name = true;
>    common = commands/true.c;
>  };
>
> === added file 'grub-core/commands/testpci.c'
> --- grub-core/commands/testpci.c        1970-01-01 00:00:00 +0000
> +++ grub-core/commands/testpci.c        2013-01-23 22:21:11 +0000
> @@ -0,0 +1,128 @@
> +/* testpci.c - Test for PCI device existence.  */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2013 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/pci.h>
> +#include <grub/dl.h>
> +#include <grub/extcmd.h>
> +#include <grub/misc.h>
> +#include <grub/i18n.h>
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +static const struct grub_arg_option options[] =
> +  {
> +    {"vendor", 'v' , 0, N_("PCI vendor ID to look for"), 0, ARG_TYPE_STRING},
> +    {"product", 'p' , 0, N_("PCI product ID to look for"), 0, 
> ARG_TYPE_STRING},
> +    {"subvendor", 's' , 0, N_("PCI subsystem vendor ID to look for"), 0, 
> ARG_TYPE_STRING},
> +    {"subproduct", 't' , 0, N_("PCI subsystem product ID to look for"), 0, 
> ARG_TYPE_STRING},
> +    {0, 0, 0, 0, 0, 0}
> +  };
> +
> +static grub_uint32_t pcimask, pcisubmask;
> +static grub_pci_id_t pcisearch;
> +static grub_pci_id_t pcisubsearch;
> +
> +static int
> +grub_testpci_iter (grub_pci_device_t dev __attribute__ ((unused)),
> +                  grub_pci_id_t pciid,
> +                  void *data)
> +{
> +  grub_pci_address_t addr;
> +  grub_uint32_t subsystem;
> +  int *found = data;
> +
> +  if ((pciid & pcimask) != pcisearch) {
> +    return 0;
> +  }
> +
> +  if (pcisubmask != 0) {
> +    addr = grub_pci_make_address (dev, GRUB_PCI_REG_SUBVENDOR);
> +    subsystem = grub_pci_read (addr);
> +    if ((subsystem & pcisubmask) != pcisubsearch) {
> +      return 0;
> +    }
> +  }
> +
> +  *found = 1;
> +  return 1;
> +}
> +
> +static grub_err_t
> +grub_cmd_testpci (grub_extcmd_context_t ctxt,
> +                 int argc __attribute__ ((unused)),
> +                 char **args __attribute__ ((unused)))
> +{
> +  struct grub_arg_list *state = ctxt->state;
> +  int found = 0;
> +
> +  pcimask = pcisubmask = pcisearch = pcisubsearch = 0;
> +
> +  if (state[0].set) {
> +    pcisearch |= grub_strtoull (state[0].arg, 0, 16);
> +    if (grub_errno != GRUB_ERR_NONE) {
> +      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Couldn't parse PCI 
> vendor ID"));
> +    }
> +    pcimask |= 0xFFFF;
> +  }
> +
> +  if (state[1].set) {
> +    pcisearch |= (grub_strtoull (state[1].arg, 0, 16) << 16);
> +    if (grub_errno != GRUB_ERR_NONE) {
> +      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Couldn't parse PCI 
> product ID"));
> +    }
> +    pcimask |= 0xFFFF0000;
> +  }
> +
> +  if (state[2].set) {
> +    pcisubsearch |= grub_strtoull (state[2].arg, 0, 16);
> +    if (grub_errno != GRUB_ERR_NONE) {
> +      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Couldn't parse PCI 
> subsystem vendor ID"));
> +    }
> +    pcisubmask |= 0xFFFF;
> +  }
> +
> +  if (state[3].set) {
> +    pcisubsearch |= (grub_strtoull (state[3].arg, 0, 16) << 16);
> +    if (grub_errno != GRUB_ERR_NONE) {
> +      return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("Couldn't parse PCI 
> subsystem product ID"));
> +    }
> +    pcisubmask |= 0xFFFF0000;
> +  }
> +
> +  grub_pci_iterate (grub_testpci_iter, &found);
> +
> +  return found ? GRUB_ERR_NONE : grub_error (GRUB_ERR_TEST_FAILURE, "false");
> +}
> +
> +static grub_extcmd_t cmd;
> +
> +GRUB_MOD_INIT(testpci)
> +{
> +  cmd = grub_register_extcmd ("testpci", grub_cmd_testpci, 0,
> +                             N_("[--vendor=VENDORID] [--product=PRODUCTID] "
> +                                "[--subvendor=VENDORID] "
> +                                "[--subproduct=PRODUCTID]"),
> +                             N_("Test for PCI device existence."),
> +                             options);
> +}
> +
> +GRUB_MOD_FINI(testpci)
> +{
> +  grub_unregister_extcmd (cmd);
> +}
>
> -----
>
> J.
>
> --
> "Basically, if you're allowed to own it, you're probably allowed to
> burn it." -- Stephen Gower, ox.general
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iQIcBAEBCAAGBQJRAhRJAAoJEP8WL8XPP7rRtKsP/jubezexP4PNc6VDQSYyYjD+
> GUDQ6W84rFJu3UhoWUFBBBCHHyw9d/bBzzi/52kVmskrMw/KxP8pEe+fz70//4rq
> rlIlgCWvgvL3c32cHQrBlBIk1eiNKby9Go1j/u+kqa+bDE03XHyY6yy6pWKQ7wuO
> 1Kx0EPYSarLSE5Ds1q8vdoljdQgBBq/B/aZb579HHC5a4W635ZtkSid+RPNuSylO
> kxjdTdHoJzw73FW9XUCd1DjBSXMhSp5QCbDfYeCtbhpIv8mis6mgWrcp1+EwgSWL
> Viv52Xm4OIjwXK8r3uKmf2jHp5PO4iJmmYP3cER0RPIHORKjGM1nszqtgSBj1gK/
> upgWPD/D1COu0r1Bgs9wlNXq/MESzwQaAqp+I1wpZUSlGWV7kuymIros3/Y16wgV
> n1glHfh4SlAlORo0VzhDv5r4hwZAjgWtlhT2pFR3ikWml5TzJkX1QHAlQxoeNL7Z
> fUiWJ4lgdfi6uqrMGzJ4OGYuMBWOGhUMaxkBwS4vRgEen5BB3/dLxaYKJ8Be1d6U
> K49dAYnf2dccpKRerECkTYCkrMS+FJPgYIamfHFVwd4me1zj9WyXwUbaH/ROYHF6
> TAkxl4s5W8lj7M15v76qA1o+yhY2cDoTJq+kPD+0HksDaxRqjoWMmhEUdHJtx8K5
> zdtgmocVpi54pwhPBxn8
> =wf4i
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>