Message ID | 20220208064601.237582-1-damien.lemoal@opensource.wdc.com (mailing list archive) |
---|---|
State | Handled Elsewhere, archived |
Headers | show |
Series | ata: sata_fsl: fix sscanf() and sysfs_emit() format strings | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
On 2/8/22 15:46, Damien Le Moal wrote: > Use the %u format for unsigned int parameters handling with sscanf() and > sysfs_emit() to avoid compilation warnings. In > fsl_sata_rx_watermark_store(), the call to sscanf() to parse a single > argument is replaced with a call to kstrtouint(). > > While at it, also replace the printk(KERN_ERR) calls with dev_err() > calls and fix blank lines in fsl_sata_rx_watermark_store(). > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > --- > drivers/ata/sata_fsl.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c > index da0152116d9f..556034a15430 100644 > --- a/drivers/ata/sata_fsl.c > +++ b/drivers/ata/sata_fsl.c > @@ -322,7 +322,7 @@ static void fsl_sata_set_irq_coalescing(struct ata_host *host, > static ssize_t fsl_sata_intr_coalescing_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - return sysfs_emit(buf, "%d %d\n", > + return sysfs_emit(buf, "%u %u\n", > intr_coalescing_count, intr_coalescing_ticks); > } > > @@ -332,10 +332,8 @@ static ssize_t fsl_sata_intr_coalescing_store(struct device *dev, > { > unsigned int coalescing_count, coalescing_ticks; > > - if (sscanf(buf, "%d%d", > - &coalescing_count, > - &coalescing_ticks) != 2) { > - printk(KERN_ERR "fsl-sata: wrong parameter format.\n"); > + if (sscanf(buf, "%u%u", &coalescing_count, &coalescing_ticks) != 2) { PPC folks, The "%u%u" sscanf() format above seems totally bogus to me. How could 2 unsigned int without a separating characters be parsed as such ? Surely, a separation character is needed, no ? I cannot find any documentation on what the intr_coalescing sysfs attribute format should be... Please have a look. > + dev_err(dev, "fsl-sata: wrong parameter format.\n"); > return -EINVAL; > } > > @@ -359,7 +357,7 @@ static ssize_t fsl_sata_rx_watermark_show(struct device *dev, > rx_watermark &= 0x1f; > spin_unlock_irqrestore(&host->lock, flags); > > - return sysfs_emit(buf, "%d\n", rx_watermark); > + return sysfs_emit(buf, "%u\n", rx_watermark); > } > > static ssize_t fsl_sata_rx_watermark_store(struct device *dev, > @@ -373,8 +371,8 @@ static ssize_t fsl_sata_rx_watermark_store(struct device *dev, > void __iomem *csr_base = host_priv->csr_base; > u32 temp; > > - if (sscanf(buf, "%d", &rx_watermark) != 1) { > - printk(KERN_ERR "fsl-sata: wrong parameter format.\n"); > + if (kstrtouint(buf, 10, &rx_watermark) < 0) { > + dev_err(dev, "fsl-sata: wrong parameter format.\n"); > return -EINVAL; > } > > @@ -382,8 +380,8 @@ static ssize_t fsl_sata_rx_watermark_store(struct device *dev, > temp = ioread32(csr_base + TRANSCFG); > temp &= 0xffffffe0; > iowrite32(temp | rx_watermark, csr_base + TRANSCFG); > - > spin_unlock_irqrestore(&host->lock, flags); > + > return strlen(buf); > } >
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c index da0152116d9f..556034a15430 100644 --- a/drivers/ata/sata_fsl.c +++ b/drivers/ata/sata_fsl.c @@ -322,7 +322,7 @@ static void fsl_sata_set_irq_coalescing(struct ata_host *host, static ssize_t fsl_sata_intr_coalescing_show(struct device *dev, struct device_attribute *attr, char *buf) { - return sysfs_emit(buf, "%d %d\n", + return sysfs_emit(buf, "%u %u\n", intr_coalescing_count, intr_coalescing_ticks); } @@ -332,10 +332,8 @@ static ssize_t fsl_sata_intr_coalescing_store(struct device *dev, { unsigned int coalescing_count, coalescing_ticks; - if (sscanf(buf, "%d%d", - &coalescing_count, - &coalescing_ticks) != 2) { - printk(KERN_ERR "fsl-sata: wrong parameter format.\n"); + if (sscanf(buf, "%u%u", &coalescing_count, &coalescing_ticks) != 2) { + dev_err(dev, "fsl-sata: wrong parameter format.\n"); return -EINVAL; } @@ -359,7 +357,7 @@ static ssize_t fsl_sata_rx_watermark_show(struct device *dev, rx_watermark &= 0x1f; spin_unlock_irqrestore(&host->lock, flags); - return sysfs_emit(buf, "%d\n", rx_watermark); + return sysfs_emit(buf, "%u\n", rx_watermark); } static ssize_t fsl_sata_rx_watermark_store(struct device *dev, @@ -373,8 +371,8 @@ static ssize_t fsl_sata_rx_watermark_store(struct device *dev, void __iomem *csr_base = host_priv->csr_base; u32 temp; - if (sscanf(buf, "%d", &rx_watermark) != 1) { - printk(KERN_ERR "fsl-sata: wrong parameter format.\n"); + if (kstrtouint(buf, 10, &rx_watermark) < 0) { + dev_err(dev, "fsl-sata: wrong parameter format.\n"); return -EINVAL; } @@ -382,8 +380,8 @@ static ssize_t fsl_sata_rx_watermark_store(struct device *dev, temp = ioread32(csr_base + TRANSCFG); temp &= 0xffffffe0; iowrite32(temp | rx_watermark, csr_base + TRANSCFG); - spin_unlock_irqrestore(&host->lock, flags); + return strlen(buf); }
Use the %u format for unsigned int parameters handling with sscanf() and sysfs_emit() to avoid compilation warnings. In fsl_sata_rx_watermark_store(), the call to sscanf() to parse a single argument is replaced with a call to kstrtouint(). While at it, also replace the printk(KERN_ERR) calls with dev_err() calls and fix blank lines in fsl_sata_rx_watermark_store(). Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> --- drivers/ata/sata_fsl.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)