Re: [PATCH] staging: dgap: fix kernel oops on port open

2014-02-26 Thread Dan Carpenter 
On Wed, Feb 26, 2014 at 08:41:38AM -0500, Mark Hounschell wrote:
 -static void dgap_sysfs_create(struct board_t *brd)
 +static int dgap_tty_register_ports(struct board_t *brd)
  {
   struct channel_t *ch;
 - int j = 0;
 + int i;
 +
 + brd-SerialPorts = kcalloc(brd-nasync, sizeof(*brd-SerialPorts),
 + GFP_KERNEL);
 + if (brd-SerialPorts == NULL) {
 + pr_err(dgap: Cannot allocate serial port memory\n);

Don't add these error messages here.  kcalloc() already prints a much
better error message.

 + return -ENOMEM;
 + }
 + for (i = 0; i  brd-nasync; i++)
 + tty_port_init(brd-SerialPorts[i]);
 +
 + brd-PrinterPorts = kcalloc(brd-nasync, sizeof(*brd-PrinterPorts),
 + GFP_KERNEL);
 + if (brd-PrinterPorts == NULL) {
 + pr_err(dgap: Cannot allocate printer port memory\n);

kfree(brd-SerialPorts);

 + return -ENOMEM;
 + }
 + for (i = 0; i  brd-nasync; i++)
 + tty_port_init(brd-PrinterPorts[i]);
  
   ch = brd-channels[0];
 - for (j = 0; j  brd-nasync; j++, ch = brd-channels[j]) {
 + for (i = 0; i  brd-nasync; i++, ch = brd-channels[i]) {
   struct device *classp;

Put a blank line after the declaration block.

 - classp = tty_register_device(brd-SerialDriver, j,
 - (ch-ch_bd-pdev-dev));
 +
 + classp = tty_port_register_device(brd-SerialPorts[i],
 + brd-SerialDriver, brd-firstminor + i, NULL);


The more traditional way to break up the long lines is:

classp = tty_port_register_device(brd-SerialPorts[i],
  brd-SerialDriver,
  brd-firstminor + i, NULL);
dgap_create_tty_sysfs(ch-ch_tun, classp);
ch-ch_tun.un_sysfs = classp;

classp = tty_port_register_device(brd-PrinterPorts[i],
  brd-PrintDriver,
  brd-firstminor + i,
  NULL);
dgap_create_tty_sysfs(ch-ch_pun, classp);
ch-ch_pun.un_sysfs = classp;

(also i re-ordered the last two lines).

(also I wouldn't have sent this email at all if there hadn't been that
missing kfree()).

regards,
dan carpenter

 +
   ch-ch_tun.un_sysfs = classp;
   dgap_create_tty_sysfs(ch-ch_tun, classp);
  
 - classp = tty_register_device(brd-PrintDriver, j,
 - (ch-ch_bd-pdev-dev));
 + classp = tty_port_register_device(brd-PrinterPorts[i],
 + brd-PrintDriver, brd-firstminor + i, NULL);
 +
   ch-ch_pun.un_sysfs = classp;
   dgap_create_tty_sysfs(ch-ch_pun, classp);
   }
   dgap_create_ports_sysfiles(brd);
 +
 + return 0;

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: dgap: fix kernel oops on port open

2014-02-26 Thread Mark Hounschell 

On 02/26/2014 09:01 AM, Dan Carpenter wrote:

On Wed, Feb 26, 2014 at 08:41:38AM -0500, Mark Hounschell wrote:

-static void dgap_sysfs_create(struct board_t *brd)
+static int dgap_tty_register_ports(struct board_t *brd)
  {
struct channel_t *ch;
-   int j = 0;
+   int i;
+
+   brd-SerialPorts = kcalloc(brd-nasync, sizeof(*brd-SerialPorts),
+   GFP_KERNEL);
+   if (brd-SerialPorts == NULL) {
+   pr_err(dgap: Cannot allocate serial port memory\n);


Don't add these error messages here.  kcalloc() already prints a much
better error message.



OK


+   return -ENOMEM;
+   }
+   for (i = 0; i  brd-nasync; i++)
+   tty_port_init(brd-SerialPorts[i]);
+
+   brd-PrinterPorts = kcalloc(brd-nasync, sizeof(*brd-PrinterPorts),
+   GFP_KERNEL);
+   if (brd-PrinterPorts == NULL) {
+   pr_err(dgap: Cannot allocate printer port memory\n);


kfree(brd-SerialPorts);


+   return -ENOMEM;
+   }
+   for (i = 0; i  brd-nasync; i++)
+   tty_port_init(brd-PrinterPorts[i]);

ch = brd-channels[0];
-   for (j = 0; j  brd-nasync; j++, ch = brd-channels[j]) {
+   for (i = 0; i  brd-nasync; i++, ch = brd-channels[i]) {
struct device *classp;


Put a blank line after the declaration block.



OK


-   classp = tty_register_device(brd-SerialDriver, j,
-   (ch-ch_bd-pdev-dev));
+
+   classp = tty_port_register_device(brd-SerialPorts[i],
+   brd-SerialDriver, brd-firstminor + i, NULL);



The more traditional way to break up the long lines is:

classp = tty_port_register_device(brd-SerialPorts[i],
  brd-SerialDriver,
  brd-firstminor + i, NULL);
dgap_create_tty_sysfs(ch-ch_tun, classp);
ch-ch_tun.un_sysfs = classp;

classp = tty_port_register_device(brd-PrinterPorts[i],
  brd-PrintDriver,
  brd-firstminor + i,
  NULL);
dgap_create_tty_sysfs(ch-ch_pun, classp);
ch-ch_pun.un_sysfs = classp;

(also i re-ordered the last two lines).



OK again.


(also I wouldn't have sent this email at all if there hadn't been that
missing kfree()).

regards,
dan carpenter


Thanks. I'll fix and resend

Mark

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel