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: <20060912011346.GB5192@martell.zuzino.mipt.ru>
Date:	Tue, 12 Sep 2006 05:13:46 +0400
From:	Alexey Dobriyan <adobriyan@...il.com>
To:	Miguel Ojeda <maxextreme@...il.com>
Cc:	akpm@...l.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] display: Driver ks0108 and cfag12864b

On Tue, Sep 12, 2006 at 01:57:23AM +0200, Miguel Ojeda wrote:
> +int cfag12864b_init(void)

arrrrggg....  I'm in the middle of reading every module_init and every
module_exit func, and this starts getting really annoying....

	1. module_init function SHOULD be written as

		static int __init FOO_init_module(void)
		{
			...
		}

	2. module_exit function SHOULD be written as

		static void __exit FOO_exit_module(void)
		{
			..
		}

	3. If one of function during module initialization returns an
	   error, everything done so far MUST be backed out to not cause
	   leaks.

	4. module_init and module_exit functions SHOULD NOT spit useless
	   messages upon which system administrator can't act meaningfully.

	5. In case of error during initialization, error code SHOULD be
	   propagated as is to upper layers, either via direct
	   assignment/return or via decoding from pointer.

	6. Error messages SHOULD start with short unique prefix specific to
	   driver. Module name without .o and .ko is fine.

	7. StupidCapitalization MUST NOT be used.

	8. Function args SHOULD NOT be prefixed with underscore without
	   reason.

	9. Various sorts of operations and methods driver provides to
	   upper layers SHOULD start with short, common to driver
	   prefix and SHOULD be made static where possible:

		static struct foobar_operations rtl8139_fops = {
			...
		};

> +{
> +       unsigned int i;
> +       int Result;
> +
> +       printk(PRINTK_PREFIX "Init... ");
> +
> +       Result = alloc_chrdev_region(&FirstDevice, FirstMinor, nDevices, 
> Name);
> +       if(Result < 0) {
> +               printk("ERROR - alloc_chrdev_region\n");
> +               return Result;
> +       }
> +       Major = MAJOR(FirstDevice);
> +
> +       Devices = kmalloc(nDevices * sizeof(struct cfag12864b), GFP_KERNEL);
> +       if(Devices == NULL) {
> +               printk("ERROR - kmalloc\n");
> +               return -ENOMEM;
> +       }
> +       memset(Devices, 0, nDevices * sizeof(struct cfag12864b));
> +
> +       for(i=0; i<nDevices; ++i) {
> +               Devices[i].Minor = FirstMinor+i;
> +               Devices[i].Device = MKDEV(Major,Devices[i].Minor);
> +
> +               cdev_init(&(Devices[i].CharDevice), &Fops);
> +               Devices[i].CharDevice.owner = THIS_MODULE;
> +               Devices[i].CharDevice.ops = &Fops;
> +               Result = cdev_add(&(Devices[i].CharDevice),
> +                       Devices[i].Device, 1);
> +               if(Result < 0) {
> +                       printk("ERROR - cdev_add\n");
> +                       kfree(Devices);
> +                       return Result;
> +               }
> +
> +               class_device_create(
> +                       display_class,NULL,MKDEV(Major,Devices[i].Minor),
> +                       NULL,"cfag12864b%d",Devices[i].Minor);
> +       }
> +
> +
> +       cfag12864b_Clear();
> +       cfag12864b_On();

	->open, ->write could be called as soon as cdev_add succeeds.
	Is hardware functional at that point?

-
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