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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 23 May 2012 22:07:16 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	H Hartley Sweeten <hartleys@...ionengravers.com>
Cc:	Ian Abbott <abbotti@....co.uk>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"fmhess@...rs.sourceforge.net" <fmhess@...rs.sourceforge.net>,
	Ian Abbott <ian.abbott@....co.uk>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] staging: comedi: remove this_board macro in the s526
 driver

On Wed, May 23, 2012 at 11:28:32AM -0500, H Hartley Sweeten wrote:
> On Wednesday, May 23, 2012 2:18 AM, Ian Abbott wrote:
> > On 2012-05-23 06:48, Dan Carpenter wrote:
> >> On Tue, May 22, 2012 at 06:20:10PM -0700, H Hartley Sweeten wrote:
> >>> -/*
> >>> - * Initialize dev->board_name.  Note that we can use the "thisboard"
> >>> - * macro now, since we just initialized it in the last line.
> >>> - */
> >>> -	dev->board_ptr =&s526_boards[0];
> >>
> >> Was this intended?  Most of the boards have auto probing so the
> >> ->board_ptr gets set automatically.  We already called
> >> comedi_board() so I wonder if the autoprobed board is the same as
> >> the&s526_boards[0];?  NULL pointer perhaps?  I don't know.
> >
> > If .num_names in struct comedi_driver is non-zero, the core comedi 
> > module will set dev->board_ptr to something non-NULL and meaningful to 
> > the driver before it calls the driver's ->attach() method.  (This is 
> > done by the comedi_recognize() function.)  The driver can change 
> > dev->board_ptr to something else if it wants.  In this case, 
> > dev->board_ptr will already be set to &s526_boards[0] so there is no 
> > point setting it again.
> 
> Ian,
> 
> Side-note on the boardinfo stuff.
> 
> I wonder if it makes sense to create a common boardinfo struct
> and modify the comedi_driver struct a bit. Something like:
> 
> struct comedi_boardinfo {
> 	const char *name;
> 	unsigned short	vendor;
> 	unsigned short device;
> 	void *private;
> };
> 
> struct comedi_driver {
> 	struct comedi_driver *next;
> 
> 	const char *driver_name;
> 	struct module *module;
> 	int (*attach) (struct comedi_device *, struct comedi_devconfig *);
> 	void (*detach) (struct comedi_device *);
> 	int (*attach_pci) (struct comedi_device *, struct pci_dev *);
> 	int (*attach_usb) (struct comedi_device *, struct usb_interface *);
> 
> 	u32 num_boards;
> 	struct comedi_boardinfo *board;
> };
> 
> This would allow comedi_recognize() to walk the boardinfo
> to find the match without all the ugly casts.

That function wouldn't be so bad if we just removed all the consts.
There is no point because we drop all of them anyway when we return
the struct.

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ