Re: [KJ] [PATCH] printk function

[Alexey Dobriyan <adobriyan@mail.ru>]

[-- Attachment #1: Type: text/plain, Size: 5265 bytes --]

On Tuesday 18 January 2005 02:26, Rafael Pereira wrote:

> The patch for the printk function.

> --- linux-2.6.11-rc1/arch/alpha/kernel/core_wildfire.c
> +++ linux/arch/alpha/kernel/core_wildfire.c

> -	printk(" hard_qbb_map: ");
> +	printk(KERN_ERR " hard_qbb_map: ");

Notice this printk doesn't have trailing "\n".

>  	for (i = 0; i < WILDFIRE_MAX_QBB; i++)
>  	    if (wildfire_hard_qbb_map[i] == QBB_MAP_EMPTY)
> -		printk("--- ");
> +		printk(KERN_ERR "--- ");

Adding KERN_* in such loops screwes the output.

>  	    else
> -		printk("%3d ", wildfire_hard_qbb_map[i]);
> -	printk("\n");
> +		printk(KERN_ERR "%3d ", wildfire_hard_qbb_map[i]);
> +	printk(KERN_ERR "\n");

Not every printk instance should use KERN_*. Domen, I think this should be
added to TODO.

> -	printk(" soft_qbb_map: ");
> +	printk(KERN_ERR " soft_qbb_map: ");
>  	for (i = 0; i < WILDFIRE_MAX_QBB; i++)
>  	    if (wildfire_soft_qbb_map[i] == QBB_MAP_EMPTY)
> -		printk("--- ");
> +		printk(KERN_ERR "--- ");
		       ^^^^^^^^
>  	    else
> -		printk("%3d ", wildfire_soft_qbb_map[i]);
> -	printk("\n");
> +		printk(KERN_ERR "%3d ", wildfire_soft_qbb_map[i]);
		       ^^^^^^^^
> +	printk(KERN_ERR "\n");
	       ^^^^^^^^

Same error.

> --- linux-2.6.11-rc1/arch/ppc64/kernel/prom.c
> +++ linux/arch/ppc64/kernel/prom.c

>  			printk(KERN_DEBUG "hmmm, got %d intr cells for %s:", n,
>  			       node->full_name);
>  			for (j = 0; j < n; ++j)
> -				printk(" %d", irq[j]);
> -			printk("\n");
> +				printk(KERN_DEBUG " %d", irq[j]);
				       ^^^^^^^^^^
> +			printk(KERN_DEBUG "\n");
			       ^^^^^^^^^^

> --- linux-2.6.11-rc1/arch/sparc64/kernel/smp.c
> +++ linux/arch/sparc64/kernel/smp.c

>  #ifdef CAPTURE_DEBUG
> -		printk("CPU[%d]: Sending penguins to jail...",
> +		printk(KERN_DEBUG "CPU[%d]: Sending penguins to jail...",
>  		       smp_processor_id());
>  #endif

>  #ifdef CAPTURE_DEBUG
> -		printk("done\n");
> +		printk(KERN_DEBUG "done\n");
		       ^^^^^^^^^^

> --- linux-2.6.11-rc1/arch/um/kernel/irq_user.c
> +++ linux/arch/um/kernel/irq_user.c

> -		printk("free_irq_later found no irq, irq = %d, "
> +		printk(fKERN_ERR "free_irq_later found no irq, irq = %d, "

Oops, a typo.

> --- linux-2.6.11-rc1/arch/um/kernel/sigio_user.c
> +++ linux/arch/um/kernel/sigio_user.c

> -	printk("Checking that host ptys support output SIGIO...");
> +	printk(KERN_WARNING "Checking that host ptys support output SIGIO...");

>  	if(got_sigio){
> -		printk("Yes\n");
> +		printk(KERN_ERR "Yes\n");
		       ^^^^^^^^
>  		pty_output_sigio = 1;
>  	}
> -	else if(n == -EAGAIN) printk("No, enabling workaround\n");
> +	else if(n == -EAGAIN) printk(KERN_ERR "No, enabling workaround\n");
				     ^^^^^^^^

>  static void tty_close(int master, int slave)
>  {
> -	printk("Checking that host ptys support SIGIO on close...");
> +	printk(KERN_INFO "Checking that host ptys support SIGIO on close...");
>  
>  	os_close_file(slave);
>  	if(got_sigio){
> -		printk("Yes\n");
> +		printk(KERN_INFO "Yes\n");
		       ^^^^^^^^^
>  		pty_close_sigio = 1;
>  	}
> -	else printk("No, enabling workaround\n");
> +	else printk(KERN_WARNING "No, enabling workaround\n");
		    ^^^^^^^^^^^^
 
> --- linux-2.6.11-rc1/drivers/block/acsi.c
> +++ linux/drivers/block/acsi.c
> @@ -1668,22 +1668,22 @@
>  				printk( KERN_INFO "Detected ");
>  				switch (aip->type) {
>  				  case HARDDISK:
> -					printk("disk");
> +					printk(KERN_INFO "disk");
					       ^^^^^^^^^
>  					break;
>  				  case CDROM:
> -					printk("cdrom");
> +					printk(KERN_INFO "cdrom");
					       ^^^^^^^^^
>  					break;
>  				  default:
>  				}
> -				printk(" ad%c at id %d lun %d ",
> +				printk(KERN_INFO " ad%c at id %d lun %d ",
>  				       'a' + NDevices, target, lun);
>  				if (aip->removable) 
> -					printk("(removable) ");
> +					printk(KERN_INFO "(removable) ");
					       ^^^^^^^^^
>  				if (aip->read_only) 
> -					printk("(read-only) ");
> +					printk(KERN_INFO "(read-only) ");
					       ^^^^^^^^^
>  				if (aip->size == DEFAULT_SIZE)
> -					printk(" unkown size, using default ");
> -				printk("%ld MByte\n",
> +					printk(KERN_INFO " unkown size, using default ");
					       ^^^^^^^^^
> +				printk(KERN_INFO "%ld MByte\n",
				       ^^^^^^^^^

> --- linux-2.6.11-rc1/kernel/module.c
> +++ linux/kernel/module.c
> @@ -2060,10 +2060,10 @@
>  {
>  	struct module *mod;
>  
> -	printk("Modules linked in:");
> +	printk(KERN_INFO "Modules linked in:");
>  	list_for_each_entry(mod, &modules, list)
> -		printk(" %s", mod->name);
> -	printk("\n");
> +		printk(KERN_INFO " %s", mod->name);
		       ^^^^^^^^^
> +	printk(KERN_INFO "\n");
	       ^^^^^^^^^

> --- linux-2.6.11-rc1/kernel/power/disk.c
> +++ linux/kernel/power/disk.c

> -	printk("Freeing memory...  ");
> +	printk(KERN_INFO "Freeing memory...  ");
>  	while ((tmp = shrink_all_memory(10000))) {
>  		pages += tmp;
> -		printk("\b%c", p[i]);
> +		printk(KERN_INFO "\b%c", p[i]);
		       ^^^^^^^^^
> -	printk("\bdone (%li pages freed)\n", pages);
> +	printk(KERN_INFO "\bdone (%li pages freed)\n", pages);
	       ^^^^^^^^^

Many more left. Double check that you aren't adding KERN_* in the _middle_ of
a log message, ok? Also minimal splitting would be nice (arch/alpha/* part,
kernel/* part, ...).

	Alexey


[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors