| 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
| ||
|
Message-ID: <2148456.iZASKD2KPV@daneel.sf-tec.de>
Date: Thu, 08 Jul 2021 23:25:37 +0200
From: Rolf Eike Beer <eike-kernel@...tec.de>
To: gregkh@...uxfoundation.org, Carlos Bilbao <bilbao@...edu>
Cc: alexander.deucher@....com, davem@...emloft.net,
mchehab+huawei@...nel.org, kuba@...nel.org,
James.Bottomley@...senpartnership.com, netdev@...r.kernel.org,
linux-parisc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers: Follow the indentation coding standard on printks
Am Donnerstag, 8. Juli 2021, 15:10:01 CEST schrieb Carlos Bilbao:
> Fix indentation of printks that start at the beginning of the line. Change
> this for the right number of space characters, or tabs if the file uses
> them.
> diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
> index bc8e8d9f176b..65bb700cd5af 100644
> --- a/drivers/atm/iphase.c
> +++ b/drivers/atm/iphase.c
> @@ -1246,7 +1246,7 @@ static void rx_intr(struct atm_dev *dev)
> ((iadev->rx_pkt_cnt - iadev->rx_tmp_cnt) == 0)) {
> for (i = 1; i <= iadev->num_rx_desc; i++)
> free_desc(dev, i);
> -printk("Test logic RUN!!!!\n");
> + printk("Test logic RUN!!!!\n");
> writew(
> ~(RX_FREEQ_EMPT|RX_EXCP_RCVD),iadev->reass_reg+REASS_MASK_REG);
> iadev->rxing = 1;
> }
This looks like leftover debug code and probably should just be deleted.
> diff --git a/drivers/atm/suni.c b/drivers/atm/suni.c
> index 21e5acc766b8..149605cdb859 100644
> --- a/drivers/atm/suni.c
> +++ b/drivers/atm/suni.c
> @@ -328,8 +328,8 @@ static int suni_start(struct atm_dev *dev)
> timer_setup(&poll_timer, suni_hz, 0);
> poll_timer.expires = jiffies+HZ;
> #if 0
> -printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long)
> poll_timer.list.prev, - (unsigned long) poll_timer.list.next);
> + printk(KERN_DEBUG "[u] p=0x%lx,n=0x%lx\n",(unsigned long)
> poll_timer.list.prev, + (unsigned long) poll_timer.list.next);
> #endif
> add_timer(&poll_timer);
> }
This should be converted to pr_debug() and the #if 0 can be removed. Or the
whole thing should likely just be removed, this looks like dead debug code.
> diff --git a/drivers/atm/zatm.c b/drivers/atm/zatm.c
> index cf5fffcf98a1..4fb89ed47311 100644
> --- a/drivers/atm/zatm.c
> +++ b/drivers/atm/zatm.c
> @@ -380,7 +380,7 @@ static void poll_rx(struct atm_dev *dev,int mbx)
> pos = zatm_dev->mbx_start[mbx];
> cells = here[0] & uPD98401_AAL5_SIZE;
> #if 0
> -printk("RX IND: 0x%x, 0x%x, 0x%x, 0x%x\n",here[0],here[1],here[2],here[3]);
> + printk("RX IND: 0x%x, 0x%x, 0x%x,
> 0x%x\n",here[0],here[1],here[2],here[3]);
> {
> unsigned long *x;
> printk("POOL: 0x%08x, 0x%08x\n",zpeekl(zatm_dev,
This is lacking a loglevel, as well as the following prints. It should be
converted to pr_debug(). The indentation of the following lines should be
fixed, too.
> @@ -403,14 +403,14 @@ EVENT("error code 0x%x/0x%x\n",(here[3] &
> uPD98401_AAL5_ES) >> skb = ((struct rx_buffer_head *)
> bus_to_virt(here[2]))->skb;
> __net_timestamp(skb);
> #if 0
> -printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx 0x%08lx\n",((unsigned *)
> skb->data)[-3],
> + printk("[-3..0] 0x%08lx 0x%08lx 0x%08lx
> 0x%08lx\n",((unsigned *) skb->data)[-3],
> ((unsigned *) skb->data)[-2],((unsigned *) skb->data)[-1],
> ((unsigned *) skb->data)[0]);
> #endif
These as well. But this doesn't make sense, the format string says %lx, but
the casts are to unsigned, so I suspect this would spit warnings if enabled.
> EVENT("skb 0x%lx, here 0x%lx\n",(unsigned long) skb,
> (unsigned long) here);
> #if 0
> -printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
> + printk("dummy: 0x%08lx, 0x%08lx\n",dummy[0],dummy[1]);
> #endif
> size = error ? 0 : ntohs(((__be16 *) skb->data)[cells*
> ATM_CELL_PAYLOAD/sizeof(u16)-3]);
Same here.
> @@ -664,7 +664,7 @@ static int do_tx(struct sk_buff *skb)
> EVENT("dsc (0x%lx)\n",(unsigned long) dsc,0);
> }
> else {
> -printk("NONONONOO!!!!\n");
> + printk("NONONONOO!!!!\n");
> dsc = NULL;
> #if 0
> u32 *put;
And this should give something more useful, at least showing the driver name
or something like that.
> diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c
> b/drivers/net/ethernet/dec/tulip/de4x5.c index b125d7faefdf..155cfe8800cd
> 100644
> --- a/drivers/net/ethernet/dec/tulip/de4x5.c
> +++ b/drivers/net/ethernet/dec/tulip/de4x5.c
> @@ -3169,7 +3169,7 @@ dc2114x_autoconf(struct net_device *dev)
>
> default:
> lp->tcount++;
> -printk("Huh?: media:%02x\n", lp->media);
> + printk("Huh?: media:%02x\n", lp->media);
> lp->media = INIT;
> break;
> }
That should be netdev_something, like netdev_dbg() or netdev_warn().
> diff --git a/drivers/net/sb1000.c b/drivers/net/sb1000.c
> index e88af978f63c..54a7c7613434 100644
> --- a/drivers/net/sb1000.c
> +++ b/drivers/net/sb1000.c
Here as well.
> --- a/drivers/parisc/iosapic.c
> +++ b/drivers/parisc/iosapic.c
> @@ -633,7 +633,7 @@ static void iosapic_unmask_irq(struct irq_data *d)
> printk("\n");
> }
>
> -printk("iosapic_enable_irq(): sel ");
> + printk("iosapic_enable_irq(): sel ");
> {
> struct iosapic_info *isp = vi->iosapic;
>
> @@ -642,7 +642,7 @@ printk("iosapic_enable_irq(): sel ");
> printk(" %x", d1);
> }
> }
> -printk("\n");
> + printk("\n");
> #endif
>
> /*
This is also debug code. It is basically unchanged since it has been imported
into git. So it may be time to remove the whole block. Helge?
> diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
> index dce4cdf786cd..c3381facdfc5 100644
> --- a/drivers/parisc/sba_iommu.c
> +++ b/drivers/parisc/sba_iommu.c
> @@ -1550,7 +1550,7 @@ static void sba_hw_init(struct sba_device *sba_dev)
>
>
> #if 0
> -printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x 0x%x\n",
> PAGE0->mem_boot.hpa,
> + printk("sba_hw_init(): mem_boot 0x%x 0x%x 0x%x
> 0x%x\n", PAGE0->mem_boot.hpa, PAGE0->mem_boot.spa, PAGE0->mem_boot.pad,
> PAGE0->mem_boot.cl_class);
>
> /*
This is equally old. It should be either also removed, also this seems at
least worth as documentation. Maybe just switch it to pr_debug() or
dev_debug() while fixing the indentation.
Eike
Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)
Powered by blists - more mailing lists