linux-kernel - Re: [PATCH v2 1/3] ACPI: Allow PCI to be disabled for reboot

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 11 Dec 2018 22:49:05 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     okaya@...nel.org
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] ACPI: Allow PCI to be disabled for reboot

On Tue, Dec 11, 2018 at 5:54 PM Sinan Kaya <okaya@...nel.org> wrote:
>
> On 12/11/2018 5:12 AM, Rafael J. Wysocki wrote:
> > On Sat, Dec 8, 2018 at 10:47 PM Sinan Kaya <okaya@...nel.org> wrote:
> >>
> >> Make PCI reboot conditional on PCI support being present on the kernel
> >> configuration.
> >>
> >> Signed-off-by: Sinan Kaya <okaya@...nel.org>
> >
> > Same comment as for patch [2/3]: make the subject say clearly that
> > this is about CONFIG_PCI.
>
> Sure
>
> >>          case ACPI_ADR_SPACE_PCI_CONFIG:
> >> +       {
> >> +#ifdef CONFIG_PCI
> >> +               unsigned int devfn;
> >> +               struct pci_bus *bus0;
> >> +
> >>                  /* The reset register can only live on bus 0. */
> >>                  bus0 = pci_find_bus(0, 0);
> >>                  if (!bus0)
> >> @@ -45,7 +48,10 @@ void acpi_reboot(void)
> >>                  pci_bus_write_config_byte(bus0, devfn,
> >>                                  (rr->address & 0xffff), reset_value);
> >>                  break;
> >> -
> >> +#else
> >> +               return;
> >
> > Why not "break"?
> >
>
> I struggled between break and return. Existing code seems to return on failure
> when bus0 is NULL. I assumed it would be more logical to return as someone could
> put some code after here that assumes everything is in order.

Well, there's no such code ATM, so there's no practical difference
between the two and you don't really need the #else branch at all, do
you?