netdev - Re: [PATCH net-next v3 02/10] ptp: ocp: add Celestica timecard PCI ids

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]

Message-ID: <20220517154506.vmic7tj7e6eddrls@bsd-mbp.dhcp.thefacebook.com>
Date:   Tue, 17 May 2022 08:45:06 -0700
From:   Jonathan Lemon <jonathan.lemon@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, richardcochran@...il.com,
        davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com,
        kernel-team@...com
Subject: Re: [PATCH net-next v3 02/10] ptp: ocp: add Celestica timecard PCI
 ids

On Tue, May 17, 2022 at 08:25:58AM -0700, Jakub Kicinski wrote:
> On Mon, 16 May 2022 18:46:44 -0700 Jonathan Lemon wrote:
> > On Mon, May 16, 2022 at 05:43:03PM -0700, Jakub Kicinski wrote:
> > > On Fri, 13 May 2022 15:59:16 -0700 Jonathan Lemon wrote:  
> > > > +#ifndef PCI_VENDOR_ID_CELESTICA
> > > > +#define PCI_VENDOR_ID_CELESTICA 0x18d4
> > > > +#endif
> > > > +
> > > > +#ifndef PCI_DEVICE_ID_CELESTICA_TIMECARD
> > > > +#define PCI_DEVICE_ID_CELESTICA_TIMECARD 0x1008
> > > > +#endif  
> > > 
> > > The ifdefs are unnecessary, these kind of constructs are often used out
> > > of tree when one does not control the headers, but not sure what purpose
> > > they'd serve upstream?  
> > 
> > include/linux/pci_ids.h says:
> > 
> >  *      Do not add new entries to this file unless the definitions
> >  *      are shared between multiple drivers.
> > 
> > Neither FACEBOOK (0x1d9b) nor CELESTICA (0x18d4) are present
> > in this file.  This seems to a common idiom in several other
> > drivers.  Picking one at random:
> > 
> >    gve.h:#define PCI_VENDOR_ID_GOOGLE 0x1ae0
> > 
> > 
> > So these #defines are needed.
> 
> Indeed, but also I'm not complaining about defines but the ifdefs 
> in which they are wrapped :)

This is standard defensive coding practice.  I would vastly
prefer to leave them this way, and my hard-wired fingers 
would also not like to be retrained.

Next, you'll be telling me that all the kernel headers
should be using "#pragma once".
-- 
Jonathan