eeds check if ap supports rx ampdu. */
> - if ((phtpriv->ampdu_enable == false) && (pregistrypriv->ampdu_enable ==
> 1)) {
> + if (!(phtpriv->ampdu_enable) && pregistrypriv->ampdu_enable == 1) {
Same.
> if (pregistrypriv->wifi_sp
; + xfi_direct_valid = xfi_indirect_valid;
> + xaui_indirect_valid = 1;
> + xaui_direct_valid = xaui_indirect_valid
The original code is fine here. Just ignore checkpatch on this.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ir heads. I suspected it might be a false positive
but I wasn't sure either way and I try not to spend a lot of time
reviewing those warnings.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Wed, Oct 09, 2019 at 04:58:12PM +0200, Alexander Gordeev wrote:
> On Wed, Oct 09, 2019 at 03:14:41PM +0300, Dan Carpenter wrote:
> > > +config AVALON_DMA_PCI_VENDOR_ID
> > > + hex "PCI vendor ID"
> > > + default "0x1172"
> > > +
> &
>> 51 ret = NULL;
> > >> 52 } else {
> > >> 53 dev_dbg(dev, "using gpio %d for %s\n",
> > >> desc_to_gpio(ret), label);
> > >> 54 }
> > >> 55
On Tue, Oct 01, 2019 at 09:58:55PM +0300, Dan Carpenter wrote:
> On Tue, Oct 01, 2019 at 06:13:21PM +0300, Denis Efremov wrote:
> > Just found an official documentation to this issue:
> > https://gcc.gnu.org/gcc-4.9/porting_to.html
> > "Null pointer checks may be optimi
Thanks!
regars,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
gt; + return 0;
> +
> +err:
> + while (!kthread_should_stop())
> + cond_resched();
> +
> + return ret;
> +}
> +
> +struct kthread_xfer_rw_sg_data {
> + struct dma_chan *chan;
> + enum dma_data_direction dir;
> + dma_addr_t dev_addr;
> + struct scatterlist *sg;
> + unsigned int sg_len;
> + void (*xfer_callback)(void *dma_async_param);
> +};
> +
> +static int __kthread_xfer_rw_sg(void *_data)
> +{
> + struct kthread_xfer_rw_sg_data *data = _data;
> +
> + return kthread_xfer_rw_sg(data->chan, data->dir,
> + data->dev_addr, data->sg, data->sg_len,
> + data->xfer_callback);
> +}
> +
> +static int __xfer_rw_sg_smp(struct dma_chan *chan,
> + enum dma_data_direction dir,
> + dma_addr_t dev_addr,
> + struct scatterlist *sg, unsigned int sg_len,
> + void (*xfer_callback)(void *dma_async_param))
> +{
> + struct kthread_xfer_rw_sg_data data = {
> + chan, dir,
> + dev_addr, sg, sg_len,
> + xfer_callback
> + };
> + struct task_struct *task;
> + struct task_struct **tasks;
> + int nr_tasks = dmas_per_cpu * num_online_cpus();
> + int n, cpu;
> + int ret = 0;
> + int i = 0;
> +
> + tasks = kmalloc(sizeof(tasks[0]) * nr_tasks, GFP_KERNEL);
kmalloc_array().
> + if (!tasks)
> + return -ENOMEM;
> +
> + for (n = 0; n < dmas_per_cpu; n++) {
> + for_each_online_cpu(cpu) {
> + if (i >= nr_tasks) {
> + ret = -ENOMEM;
> + goto kthread_err;
> + }
> +
> + task = kthread_create(__kthread_xfer_rw_sg,
> + &data, "av-dma-sg-%d-%d", cpu, n);
> + if (IS_ERR(task)) {
> + ret = PTR_ERR(task);
> + goto kthread_err;
> + }
> +
> + kthread_bind(task, cpu);
> +
> + tasks[i] = task;
> + i++;
> + }
> + }
> +
> + for (i = 0; i < nr_tasks; i++)
> + wake_up_process(tasks[i]);
> +
> + /*
> + * Run child kthreads until user sent a signal (i.e Ctrl+C)
> + * and clear the signal to avid user program from being killed.
> + */
> + schedule_timeout_interruptible(MAX_SCHEDULE_TIMEOUT);
> + flush_signals(current);
> +
> +kthread_err:
> + for (i = 0; i < nr_tasks; i++)
> + kthread_stop(tasks[i]);
This will Oops when you try free uninitialized data.
The way to do error handling is:
1) Use good label names which say what the label does. "goto free_foo;"
2) Keep track of the most recently allocated thing. "foo"
3) Free most recent thing. That will free everything and it *won't*
free things which haven't been allocated.
Here it's complicated because we have to break out of a loop. The rules
for loops are:
4) free the partial iteration before the goto.
5) free down to zero
for (i = 0; i < cnt; i++) {
foo = alloc();
if (!foo)
goto unwind_loop;
foo->bar = alloc();
if (!bar) {
free(foo);
goto unwind_loop;
}
foo->bar->baz = alloc();
if (!baz) {
free(foo->bar);
free(foo);
goto unwind_loop;
}
array[i] = foo;
}
return 0;
unwind_loop:
while (--i >= 0) {
free(array[i]->bar->baz);
free(array[i]->bar);
free(array[i]);
}
free(array);
return -ENOMEM;
My other comment would be that you're sacrificing a lot of readability
to add debug code to this driver... It's hard to review.
> +
> + kfree(tasks);
> +
> + return ret;
> +}
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ne, but keep it consistent within the
file. Here the style should be:
static int sram_write_dma_safe(struct wfx_dev *wdev, u32 addr, const u8 *buf,
size_t len)
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverp
On Wed, Oct 09, 2019 at 02:30:32PM +0300, Dan Carpenter wrote:
> > + platform = kzalloc(sizeof(*platform), GFP_KERNEL);
> > + if (!platform) {
> > + DRM_DEV_ERROR(dev, "failed to allocate driver data\n");
> > + ret = -ENOMEM;
> > +
en;
> +
> + nr_descs++;
> + if (nr_descs >= DMA_DESC_MAX)
> + break;
> +
> + desc_id++;
> + if (desc_id >= DMA_DESC_MAX)
> + break;
> +
> + de
Are you sure you sent the correct patch? This has many of the same
style issues I mentioned in the previous email. The error handling
in edid_read() is wrong. probe() will still crash if allocating the
work queue fails.
On Wed, Oct 09, 2019 at 09:28:02AM +, Xin Ji wrote:
> The ANX7625 is an
-by: kbuild test robot
Reported-by: Dan Carpenter
New smatch warnings:
drivers/staging/wfx/wfx.h:91 wdev_to_wvif() warn: potential spectre issue
'wdev->vif' [r] (local cap)
drivers/staging/wfx/data_tx.c:479 wfx_tx_get_raw_link_id() warn: signedness bug
returning &#x
G_CONFIG && cs->conf_cache >= 0)
I like these changes but Greg doesn't. So don't bother with this one.
> return cs->conf_cache;
>
> val = readq(addr);
The rest of the changes are fine. Split them into multiple patches and
resend.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
iq_2835_arm.c | 4 ++--
But this doesn't affect the final patch, and we can see all the
information so it's fine. No need to resend.
Reviewed-by: Dan Carpenter
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject
On Tue, Oct 08, 2019 at 04:21:54PM +0200, Matteo Croce wrote:
> On Tue, Oct 8, 2019 at 3:16 PM Dan Carpenter wrote:
> >
> > The subject doesn't match the patch. It should just be "remove useless
> > printk".
> >
> > regards,
> > dan ca
The subject doesn't match the patch. It should just be "remove useless
printk".
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
sizeof(msg->rx_mic_key),
"inconsistent data");
That doesn't look too good still... The error message is sort of
rubbish also. Anyway the operator goes on the first line.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
r now, but in the future there shouldn't be a
space after the cast. It's to mark that it's a high precedence
operation.
cpu_to_le32s((uint32_t *)&val);
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
.oui = { 0x50, 0x6F, 0x9A },
Please don't do unrelated white space changes in their own patches.
> }, {
> .ie_id= WLAN_EID_HT_OPERATION,
> .has_changed = 1,
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Looks good. Thanks!
Reviewed-by: Dan Carpenter
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Mon, Oct 07, 2019 at 06:15:23PM +0200, Thomas Meyer wrote:
> Dan Carpenter writes:
>
> > On Sun, Oct 06, 2019 at 04:07:45PM +0200, Thomas Meyer wrote:
> >> Use lib/crc32 instead of another implementation.
> >>
> >> Signed-off-by: Thomas Meyer
>
nlock to protect interrupt request */
But in this case, the comment isn't correct so please just leave it
as-is until someone can fix the locking.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Tue, Oct 08, 2019 at 09:06:11AM +0300, Dan Carpenter wrote:
> On Tue, Oct 08, 2019 at 01:38:58PM +0800, zhengbin wrote:
> > diff --git a/drivers/staging/sm750fb/ddk750_mode.c
> > b/drivers/staging/sm750fb/ddk750_mode.c
> > index 9722692..e0230f4 100644
> > --
ULT_INPUT_CLOCK;
> pll.clock_type = clock;
>
> - uiActualPixelClk = sm750_calc_pll_value(parm->pixel_clock, &pll);
> + sm750_calc_pll_value(parm->pixel_clock, &pll);
Get rid of this function as well.
regards,
dan carpenter
__
&m, sizeof(m), &msg_len, VCHI_FLAGS_NONE);
This looks like a bug in the code. flags is VCHI_FLAGS_NONE so it can
return -1 and we should check for that.
> if (m.type == VC_AUDIO_MSG_TYPE_RESULT) {
regards,
dan carpenter
_
-by: kbuild test robot
Reported-by: Dan Carpenter
smatch warnings:
drivers/staging/wfx/debug.c:112 wfx_send_hif_msg_read() warn: maybe return
-EFAULT instead of the bytes remaining?
#
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?id
^^
Delete these extra spaces.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
w it
because I don't have the relevant background and because I'm a little
bit stupid.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
i = _rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);
> + (void)_rtw_pktfile_read(&pktfile, (u8 *)ðerhdr, ETH_HLEN);
Don't add this "(void)" here. There is no need and it looks ugly.
_rtw_pktfile_read(&pk
On Wed, Oct 02, 2019 at 10:35:19PM +0530, Rohit Sarkar wrote:
> Now that snprintf is replaced by scnprintf n >= MAX_WPA_IE_LEN doesn't
> make sense as the maximum value n can take is MAX_WPA_IE_LEN.
>
> Signed-off-by: Rohit Sarkar
> ---
Thanks!
Reviewed-by: Dan C
Or we could apply your other patch which trumps both this patch and the
patch to the TODO.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
think that it's
not really useful. Future auditors should look for places which call
fs_set_vol_flags(sb, VOL_CLEAN); instead. That's exactly the places
which call fs_sync().
regards,
dan carpenter
___
devel mailing list
de...@linuxdr
On Wed, Oct 02, 2019 at 03:01:35PM -0400, Valdis Klētnieks wrote:
> We've seen several incorrect patches for fs_sync() calls in the exfat driver.
> Add code to the TODO that explains this isn't just a delete code and refactor,
> but that actual analysis of when the filesystem should be flushed to d
Maybe in olden times "result" used to save positive error codes instead
of negative error codes but now it's just negatives and zero on success.
There is no reason for the exit label either, we could just return
directly.
So could you redo it and get rid of "result"
direct call to bdev_sync() where
> needed and removes fs_sync definition.
>
> Signed-off-by: Saiyam Doshi
Reviewed-by: Dan Carpenter
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproj
On Wed, Oct 02, 2019 at 05:12:14PM +0530, Rohit Sarkar wrote:
> On Wed, Oct 02, 2019 at 01:57:22PM +0300, Dan Carpenter wrote:
> >
> > We could leave it as is or change it to "MAX_WPA_IE_LEN - 1". But I
> > feel like the default should be to leave it as is un
On Wed, Oct 02, 2019 at 10:03:51AM +0530, Rohit Sarkar wrote:
> On Tue, Oct 01, 2019 at 10:00:56PM +0300, Dan Carpenter wrote:
> >
> > No. scnprintf() returns the number of characters *not counting the
> > NUL terminator*. So it can be a maximum of MAX_WPA_IE_LEN - 1.
>
Someone already sent a patch to remove these functions. Generally there
should only be empty functions like this in a .h file.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman
er
> > > readability as suggested by Dan Carpenter.
> >
> > Please wrap your lines at 72 columns.
> >
> >
> I will keep this in mind. I was under the impression that the line
> length must be 80 columns
> but will make the change immediately. To be able to
On Tue, Oct 01, 2019 at 11:09:26PM +0530, Rohit Sarkar wrote:
> On Tue, Oct 01, 2019 at 11:45:14AM +0300, Dan Carpenter wrote:
> > > diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > > b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> > > index b08
ted as non-NULL.
extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
size_t __n) __THROW __nonnull ((1, 2));
We aren't going to do that in the kernel. A second difference is that
in the kernel we use -fno-delete-null-pointer-checks so it doesn&#
On Wed, Sep 11, 2019 at 12:25:03AM +0530, Rohit Sarkar wrote:
> Resending as I made a typo in Larry's email id.
>
I commented on the earlier email, but this email doesn't apply with
`git am` so it would be difficult for Larry to review it.
regard
(). In the
kernel there are lots of places which do a zero size memcpy().
The glibc attitude is "the standard allows us to put knives here" so
let's put knives everywhere in the path. And the GCC attitude is let's
silently remove NULL checks instead of just print
address this issue.
>
> Fixes: c296d5f9957c ("staging: fbtft: core support")
> Signed-off-by: Navid Emamdoost
Looks good.
Reviewed-by: Dan Carpenter
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://d
_ON() and added WARN_ON().
> Removed unnecessary parentheses:
> *(dir_entry->Name) - > *dir_entry->Name
Do these other two things in separate patches.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Sat, Sep 28, 2019 at 07:22:33PM -0500, Jesse Barton wrote:
> Fixed coding style issues with camelcase on functions and various parentheses
> that were not needed
>
Do this as two separate patches.
regards,
dan carpenter
___
devel mailin
file_id_t *fid)
I think now checkpatch will complain that the line is too long? What we
want here is:
static int ffs_create_file(struct inode *inode, char *path, u8 mode,
struct file_id_t *fid)
[tab][tab][tab][space][space][space]s
ry for HAL DATA\n");
353
> padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
> - if (!padapter->HalData)
> - DBG_88E("cant not alloc memory for HAL DATA\n");
> + if (!padapter->HalData)
ble for human beings as well. Another option
would be to just delete the "if (padapter->HalFunc.GetHwRegHandler)"
check which would also silence the false positive. A third option would
be to add "rtw_hal_get_hwreg 2" to the
It checks for overflow here. This check is impossible now and doesn't
make sense. The other loop is similar.
> break;
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Looks good. Thanks!
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Thanks!
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
nfig
> @@ -3,7 +3,7 @@ config FB_OLPC_DCON
> tristate "One Laptop Per Child Display CONtroller support"
> depends on OLPC && FB
> depends on I2C
> - depends on (GPIO_CS5535 || ACPI)
> + depends on (GPIO_CS
replies to the original
email and we apply the v1 patch that's the only thing to avoid.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
s, then this one is OK.
It has to be done in one step.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
E_MAX_H2C_BOX_NUMS;
>
> - } while ((!bcmd_down) && (retry_cnts--));
> + } while (!bcmd_down);
Just get rid of the whole do while loop, because it just goes through
one time. It doesn't loop. Get rid of bcmd_down as well.
regards,
dan carpenter
_
union iwreq_data *wrqu, char
> *extra)
> {
> - int ret = 0;
> - return ret;
> + return 0;
Someone already sent a better patch which just deletes the whole
function.
https://marc.info/?l=linux-kernel&m=156879878226402&w=2
regards,
dan
d->cmdcode, cmd_process_time);
> /* rtw_warn_on(1); */
This is indented too far now. You may was well delete the /* rtw_warn_on(1); */
line as well.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
I wish you would think more about the error codes that you're returning.
Most functions do "ret |= frob()." which ORs the error codes together,
and results in a nonsense negative error code. But then some functions
return 1 on error and zero on success which is sometimes a bug,
sometimes confusing
ions and replaced them with NULL in function array.
Thanks!
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Wed, Sep 18, 2019 at 06:53:49PM +0900, Ju Hyung Park wrote:
> Hi Dan,
>
> On Wed, Sep 18, 2019 at 6:27 PM Dan Carpenter
> wrote:
> > Put it in drivers/staging/sdfat/.
>
> It'll conflict with the current exfat staging drivers.
Use Kconfig.
> And moreover,
the fixes into
staging. The staging tree is going to have tons and tons of white space
fixes so backports are a pain. All development needs to be upstream
first where the staging driver is upstream. Otherwise we should just
wait for Samsung to get it read to be merged in fs/ and not through the
s
union iwreq_data *wrqu, char
> *extra)
> {
> - int ret = 0;
> - return ret;
> + return 0;
> }
Just get rid of the whole function. Replace the pointer in the function
array with a NULL.
regards,
dan carpenter
___
if (ret >= 0)
998 ret = comedi_device_postconfig(dev);
999 if (ret < 0) {
1000 comedi_device_detach(dev);
^
1001 module_put(driv->module);
1002 }
1003
On Sat, Sep 14, 2019 at 06:18:03PM +0300, Ivan Safonov wrote:
> On 9/10/19 2:59 PM, Dan Carpenter wrote:
> > On Sun, Sep 08, 2019 at 12:00:26PM +0300, Ivan Safonov wrote >> rtw_malloc
> > prevents the use of kmemdup/kzalloc and others.
> > >
>
You did it. Well done. :P
Reviewed-by: Dan Carpenter
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
No worries... We all have days like that occasionally. :P
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
d to highlight whitespace at the end of
a line. I don't remember how it's done now but I googled it for you.
https://vim.fandom.com/wiki/Highlight_unwanted_spaces
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ot;?
A lot of times it's really easy to see that the uses are safe, so
snprintf() is fine in that case. If it's not obviously safe then change
it.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdr
et a lot of stalls, then that looks like it could lead to a
read overflow (an information leak). Either way this does make the
code a bit easier to audit so it seems like a nice cleanup. Next time
though, I really would prefer if you put this sort analysis in yo
^^^
Get rid of this.
> return FFS_INVALIDPATH;
>
> - strcpy(name_buf, path);
> + if (strscpy(name_buf, path, sizeof(name_buf)) < 0)
> + return FFS_INVALIDPATH;
regards,
dan
On Wed, Sep 11, 2019 at 12:24:23PM +0200, Sandro Volery wrote:
>
>
> > On 11 Sep 2019, at 12:06, Dan Carpenter wrote:
> >
> > On Wed, Sep 11, 2019 at 11:42:19AM +0200, Sandro Volery wrote:
> >> Use strscpy instead of strcpy in exfat_core.c, and add a check
On Wed, Sep 11, 2019 at 11:21:44AM +0200, Sandro Volery wrote:
>
> On 11 Sep 2019, at 11:17, Dan Carpenter wrote:
> >
> > On Wed, Sep 11, 2019 at 11:04:38AM +0200, Sandro Volery wrote:
> >>
> >>
> >>>> On 11 Sep 2019, at 10:52, Dan Carpent
^^
Delete this as it is no longer required.
> return FFS_INVALIDPATH;
>
> - strcpy(name_buf, path);
> + if (strscpy(name_buf, path, sizeof(name_buf)) < 0)
> + return FFS_INVALIDPATH;
regards,
dan ca
On Wed, Sep 11, 2019 at 11:04:38AM +0200, Sandro Volery wrote:
>
>
> > On 11 Sep 2019, at 10:52, Dan Carpenter wrote:
> >
> > On Wed, Sep 11, 2019 at 08:23:59AM +0200, Sandro Volery wrote:
> >> strcpy was used multiple times in strcpy to write into dev->nam
"pow%d");
> + strscpy(dev->name, "pow%d", sizeof(dev->name));
Is there a program which is generating a warning for this code? We know
that "pow%d" is 6 characters and static analysis tools can understand
this code fine so we kn
wow, the "name_buf" is a shared buffer. wow wow wow. This seems very
racy.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Tue, Sep 10, 2019 at 01:58:35PM +0100, Colin Ian King wrote:
> On 10/09/2019 13:44, Dan Carpenter wrote:
> > On Fri, Aug 30, 2019 at 07:38:00PM +0100, Colin Ian King wrote:
> >> Hi,
> >>
> >> Static analysis on exfat with Coverity has picked up an assignment
= p_fs->root_dir) &&
> 1752(fid->entry == -1)) {
> 1753if (p_fs->dev_ejected)
Idealy we would have both a filename and a function name but this email
doesn't have either so no one knows what
Looks good.
Reviewed-by: Dan Carpenter
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
e = kmalloc(remainder_ielen, in_interrupt()
> ? GFP_ATOMIC : GFP_KERNEL);
^
This stuff isn't right. It really should be checking if spinlocks are
held or IRQs are disabled. And the only way to do that is by auditing
the callers.
(The
ame, MAX_HEAP_NAME);
> - hdata.name[sizeof(hdata.name) - 1] = '\0';
> + stracpy(hdata.name, heap->name, MAX_HEAP_NAME);
stracpy() only takes two arguments. This doesn't compile.
regards,
dan carpenter
___
d
(
> request->channels[i]->center_freq);
No, that looks even worse.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
t signing your paychecks.
Take as long as you want. :P
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
10 Sep 2019, at 07:06, Sandro Volery wrote:
> >
> > Using temporaries for gasket_page_table entries to remove scnprintf()
> > statements and reduce line length, as suggested by Joe Perches. Thanks!
^^^^^^
On Sat, Sep 07, 2019 at 04:48:21PM +0200, Sandro Volery wrote:
> > Joe's comments are, of course, correct as well.
> >
> > regards,
> > dan carpenter
> >
>
> I'll take a look at Joe's suggestions too sometime tonight. I'd feel kinda
&
ut off line
>
> Signed-off-by: Sandro Volery
> ---
^^^
Put it here. It will be removed when we apply the patch so it won't
be recorded in the git log.
> drivers/staging/gasket/apex_driver.c | 9 ++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
Joe's comments
It would be better to remove "n" altogether.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
These two error paths need to unlock before we can return.
Signed-off-by: Dan Carpenter
---
drivers/staging/exfat/exfat_super.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/exfat/exfat_super.c
b/drivers/staging/exfat/exfat_super.c
index
We don't really require benchmarks, just that a reasonable person would
think it might make a difference.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Fri, Aug 30, 2019 at 02:25:13PM +0800, Chao Yu wrote:
> On 2019/8/30 11:36, Gao Xiang wrote:
> > As Dan Carpenter suggested [1], I have to remove
> > all erofs likely/unlikely annotations.
> >
> > [1] https://lore.kernel.org/linux-fsdevel/20190829154346.GK23584@
On Fri, Aug 30, 2019 at 11:36:42AM +0800, Gao Xiang wrote:
> As Dan Carpenter suggested [1], I have to remove
> all erofs likely/unlikely annotations.
>
> [1] https://lore.kernel.org/linux-fsdevel/20190829154346.GK23584@kadam/
> Reported-by: Dan Carpenter
> Signed
On Fri, Aug 30, 2019 at 04:43:33PM +0800, Gao Xiang wrote:
> Hi Dan,
>
> On Fri, Aug 30, 2019 at 11:34:45AM +0300, Dan Carpenter wrote:
> > On Fri, Aug 30, 2019 at 12:04:41AM +0800, Gao Xiang wrote:
> > > Anyway, I'm fine to delete them all if you like, but I t
ng to be expensive. The likely/unlikely annotations
should be used in places a reasonable person thinks it will make a
difference to benchmarks.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
bout so I'm fine with removing all of them in one go.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
On Thu, Aug 29, 2019 at 01:18:10PM +0200, Greg Kroah-Hartman wrote:
> It could always use more review, which the developers asked for
> numerous times.
I stopped commenting on erofs style because all the likely/unlikely()
nonsense is a pet peeve.
regards,
dan car
oes "sensitive" mean here? Now that it's out of staging we
aren't applying clean up patches?
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
quot;.
> and I will review it then.
>
> p.s. if someone argues here or there, there will be endless
> issues since there is no standard at all.
More complaining... It doesn't matter if you can find ext4 that looks
like dog poop, that's irrelevant.
regards,
dan carpenter
___
e handled this part fine and did not cause a use after
free but it was sort of complicated to read.
Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
Signed-off-by: Dan Carpenter
---
drivers/staging/greybus/light.c | 12 ++--
1 file changed, 6 insertions(+), 6
601 - 700 of 4832 matches
Mail list logo
