Re: Personal Assistant and Administrative officer needed^
Hello, I'm looking for someone who can handle my business & personal errands at his/her spare time as I keep traveling a lot. Someone who can offer me these services mentioned below: * Mail services (Receive my mails and drop them off at UPS or USPS) * Shop for Gifts * Bill payment (pay my bills on my behalf, access to the funds would be provided by me) * Sit for delivery (at your home) or pick items up at nearby post office at your convenience. Let me know if you will be able to offer me any or all of these services and 10% of my income weekly would be your weekly payment. If you will be available for this job position ,send me a confirmation e-mail and send me your details like complete name/address/country/state/ city/zip/phone or you could even attach your resume.I do have a pile up of work and a number of unattended duties which you can assist me with soon. Please note that this job DOES NOT require any financial obligation of any sort from you as I would be catering for all expenses. I look forward to hearing from you. Sincerely, Mr.Walter.Smith. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] gs_fpgaboot: add SPDX-License-Identifier
Reported by checkpatch. Signed-off-by: Lukas Walter --- drivers/staging/gs_fpgaboot/gs_fpgaboot.c | 1 + drivers/staging/gs_fpgaboot/gs_fpgaboot.h | 1 + drivers/staging/gs_fpgaboot/io.c | 1 + drivers/staging/gs_fpgaboot/io.h | 1 + 4 files changed, 4 insertions(+) diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c index fa8b27e091a2..36a0a59579e4 100644 --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0+ /* * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h index 986e841f6b5e..59fd05b510cf 100644 --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.h +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ /* * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/drivers/staging/gs_fpgaboot/io.c b/drivers/staging/gs_fpgaboot/io.c index 83a13ca7259a..7de43179943d 100644 --- a/drivers/staging/gs_fpgaboot/io.c +++ b/drivers/staging/gs_fpgaboot/io.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0+ /* * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by diff --git a/drivers/staging/gs_fpgaboot/io.h b/drivers/staging/gs_fpgaboot/io.h index bc5d99cbda8f..9f7b6d282249 100644 --- a/drivers/staging/gs_fpgaboot/io.h +++ b/drivers/staging/gs_fpgaboot/io.h @@ -1,3 +1,4 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ /* * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
AW: [PATCH] staging: gasket: Fix sizeof() in gasket_handle_ioctl()
why not mark it as "Deprecated" and remove it with the next version ? Maybe
soneone will wakeup ?
re,
wh
Von: Greg Kroah-Hartman
Gesendet: Dienstag, 9. März 2021 14:26:55
An: Dan Carpenter
Cc: Rob Springer; de...@driverdev.osuosl.org; kernel-janit...@vger.kernel.org;
John Joseph; Simon Que; Richard Yeh; Todd Poynor
Betreff: Re: [PATCH] staging: gasket: Fix sizeof() in gasket_handle_ioctl()
On Fri, Jan 22, 2021 at 06:01:13PM +0300, Dan Carpenter wrote:
> The "gasket_dev->num_page_tables" variable is an int but this is copying
> sizeof(u64). On 32 bit systems this would end up disclosing a kernel
> pointer to user space, but on 64 bit it copies zeroes from a struct
> hole.
>
> Fixes: 9a69f5087ccc ("drivers/staging: Gasket driver framework + Apex driver")
> Signed-off-by: Dan Carpenter
> ---
> This is an API change. Please review this carefully! Another potential
> fix would be to make ->num_page_tables a long instead of an int.
>
> drivers/staging/gasket/gasket_ioctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Looks like this driver is dead, with no response from anyone from
Google.
Should I just delete it? The goal of using normal apis and getting this
out of staging seems to have totally died, so it shouldn't even still be
living in the kernel tree. Even if having it here actually finds
security issues that the authors missed like this :(
So, any objection to me deleting it?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: fix potential infinite loop because loop counter being too small
Am 01.11.2019 15:51, schrieb Dan Carpenter:
> On Fri, Nov 01, 2019 at 02:26:04PM +, Colin King wrote:
>> From: Colin Ian King
>>
>> Currently the for-loop counter i is a u8 however it is being checked
>> against a maximum value priv->ieee80211->LinkDetectInfo.SlotNum which is a
>> u16. Hence there is a potential wrap-around of counter i back to zero if
>> priv->ieee80211->LinkDetectInfo.SlotNum is greater than 255. Fix this by
>> making i a u16.
>>
>> Addresses-Coverity: ("Infinite loop")
>> Fixes: 8fc8598e61f6 ("Staging: Added Realtek rtl8192u driver to staging")
>> Signed-off-by: Colin Ian King
>> ---
>> drivers/staging/rtl8192u/r8192U_core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8192u/r8192U_core.c
>> b/drivers/staging/rtl8192u/r8192U_core.c
>> index 48f1591ed5b4..fd91b7c5ca81 100644
>> --- a/drivers/staging/rtl8192u/r8192U_core.c
>> +++ b/drivers/staging/rtl8192u/r8192U_core.c
>> @@ -3210,7 +3210,7 @@ static void rtl819x_update_rxcounts(struct r8192_priv
>> *priv, u32 *TotalRxBcnNum,
>> u32 *TotalRxDataNum)
>> {
>> u16 SlotIndex;
>> -u8 i;
>> +u16 i;
>
> The iterator "i" should just be an int unless we know that it needs to
> be an unsigned long long.
>
+1
i think we can spare the 2byte. ppl expect int and will get confused (as shown
here).
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: fix indentation issue
Am 14.11.2019 10:54, schrieb Colin King:
> From: Colin Ian King
>
> There is a block of statements that are indented
> too deeply, remove the extraneous tabs.
>
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/rtl8192u/r819xU_cmdpkt.c | 25
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> b/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> index e064f43fd8b6..bc98cdaf61ec 100644
> --- a/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> +++ b/drivers/staging/rtl8192u/r819xU_cmdpkt.c
> @@ -169,19 +169,20 @@ static void cmdpkt_beacontimerinterrupt_819xusb(struct
> net_device *dev)
> {
> struct r8192_priv *priv = ieee80211_priv(dev);
> u16 tx_rate;
> - /* 87B have to S/W beacon for DTM encryption_cmn. */
> - if (priv->ieee80211->current_network.mode == IEEE_A ||
> - priv->ieee80211->current_network.mode == IEEE_N_5G ||
> - (priv->ieee80211->current_network.mode == IEEE_N_24G &&
> - (!priv->ieee80211->pHTInfo->bCurSuppCCK))) {
> - tx_rate = 60;
> - DMESG("send beacon frame tx rate is 6Mbpm\n");
> - } else {
> - tx_rate = 10;
> - DMESG("send beacon frame tx rate is 1Mbpm\n");
> - }
>
> - rtl819xusb_beacon_tx(dev, tx_rate); /* HW Beacon */
> + /* 87B have to S/W beacon for DTM encryption_cmn. */
> + if (priv->ieee80211->current_network.mode == IEEE_A ||
> + priv->ieee80211->current_network.mode == IEEE_N_5G ||
> + (priv->ieee80211->current_network.mode == IEEE_N_24G &&
> + (!priv->ieee80211->pHTInfo->bCurSuppCCK))) {
> + tx_rate = 60;
> + DMESG("send beacon frame tx rate is 6Mbpm\n");
> + } else {
> + tx_rate = 10;
> + DMESG("send beacon frame tx rate is 1Mbpm\n");
> + }
> +
> + rtl819xusb_beacon_tx(dev, tx_rate); /* HW Beacon */
> }
>
>
> /*-
this is hard to read in the first place.
Maybe using switch() here is better to read (untested example below).
switch(priv->ieee80211->current_network.mode) {
case IEEE_A:
case IEEE_N_5G:
tx_rate = 60;
break;
IEEE_N_24G:
if ( !priv->ieee80211->pHTInfo->bCurSuppCCK )
tx_rate = 60;
// fall truh
default:
tx_rate = 10;
}
if (txrate == 60 )
DMESG("send beacon frame tx rate is 6Mbpm\n");
else if (txrate == 10 )
DMESG("send beacon frame tx rate is 1Mbpm\n");
JM2C
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: android: ion: One function call less in ion_buffer_create() after error detection
Am 27.11.2014 15:25, schrieb SF Markus Elfring:
>>> diff --git a/drivers/staging/android/ion/ion.c
>>> b/drivers/staging/android/ion/ion.c
>>> index df12cc3..7a26b85 100644
>>> --- a/drivers/staging/android/ion/ion.c
>>> +++ b/drivers/staging/android/ion/ion.c
>>> @@ -226,7 +226,7 @@ static struct ion_buffer *ion_buffer_create(struct
>>> ion_heap *heap,
>>> buffer->pages = vmalloc(sizeof(struct page *) * num_pages);
>>> if (!buffer->pages) {
>>> ret = -ENOMEM;
>>> - goto err1;
>>> + goto err2;
>>> }
>>>
>>> for_each_sg(table->sgl, sg, table->nents, i) {
>>> @@ -262,7 +262,6 @@ static struct ion_buffer *ion_buffer_create(struct
>>> ion_heap *heap,
>>> err:
>>> heap->ops->unmap_dma(heap, buffer);
>>> heap->ops->free(buffer);
>>> -err1:
>>> vfree(buffer->pages);
>>> err2:
>>
>> Now we have "err" and "err2", which doesn't make much sense,
>> please fix up.
>
> How do you want this source code to be improved?
> Should I introduce longer names for the affected jump labels?
>
hi markus,
the confusion arises because the errX are numbered and now on in the sequence
is missing.
So far i see you can renumber them or give more descriptiv names.
hope that helps,
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: bcm2835-camera: free first element in array
Am 15.02.2017 13:25, schrieb Dan Carpenter:
> We should free gdev[0] so the > should be >=.
>
> Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera
> driver.")
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> index ca15a698e018..9bcd8e546a14 100644
> --- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> @@ -1998,7 +1998,7 @@ static int __init bm2835_mmal_init(void)
> free_dev:
> kfree(dev);
>
> - for ( ; camera > 0; camera--) {
> + for ( ; camera >= 0; camera--) {
> bcm2835_cleanup_instance(gdev[camera]);
> gdev[camera] = NULL;
> }
since we already know that programmers are bad in counting backwards ...
is is possible to change that into std. loop like:
for(i=0, i< camera; i++ {
bcm2835_cleanup_instance(gdev[i]);
gdev[i] = NULL;
}
this is way a much more common pattern.
just my 2 cents,
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch v2] staging: bcm2835-camera: fix error handling in init
looks more readable now :)
Acked-by: wha...@bfs.de
Am 18.02.2017 00:20, schrieb Dan Carpenter:
> The unwinding here isn't right. We don't free gdev[0] and instead
> free 1 step past what was allocated. Also we can't allocate "dev" then
> we should unwind instead of returning directly.
>
> Fixes: 7b3ad5abf027 ("staging: Import the BCM2835 MMAL-based V4L2 camera
> driver.")
> Signed-off-by: Dan Carpenter
> ---
> v2: Change the style to make Walter Harms happy. Fix some additional
> bugs I missed in the first patch.
>
> diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> index ca15a698e018..c4dad30dd133 100644
> --- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> +++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
> @@ -1901,6 +1901,7 @@ static int __init bm2835_mmal_init(void)
> unsigned int num_cameras;
> struct vchiq_mmal_instance *instance;
> unsigned int resolutions[MAX_BCM2835_CAMERAS][2];
> + int i;
>
> ret = vchiq_mmal_init(&instance);
> if (ret < 0)
> @@ -1914,8 +1915,10 @@ static int __init bm2835_mmal_init(void)
>
> for (camera = 0; camera < num_cameras; camera++) {
> dev = kzalloc(sizeof(struct bm2835_mmal_dev), GFP_KERNEL);
> - if (!dev)
> - return -ENOMEM;
> + if (!dev) {
> + ret = -ENOMEM;
> + goto cleanup_gdev;
> + }
>
> dev->camera_num = camera;
> dev->max_width = resolutions[camera][0];
> @@ -1998,9 +2001,10 @@ static int __init bm2835_mmal_init(void)
> free_dev:
> kfree(dev);
>
> - for ( ; camera > 0; camera--) {
> - bcm2835_cleanup_instance(gdev[camera]);
> - gdev[camera] = NULL;
> +cleanup_gdev:
> + for (i = 0; i < camera; i++) {
> + bcm2835_cleanup_instance(gdev[i]);
> + gdev[i] = NULL;
> }
> pr_info("%s: error %d while loading driver\n",
> BM2835_MMAL_MODULE_NAME, ret);
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: atomisp: clean up return logic, remove redunant code
Am 11.03.2017 20:32, schrieb Colin King:
> From: Colin Ian King
>
> There is no need to check if ret is non-zero, remove this
> redundant check and just return the error status from the call
> to mt9m114_write_reg_array.
>
> Detected by CoverityScan, CID#1416577 ("Identical code for
> different branches")
>
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/media/atomisp/i2c/mt9m114.c | 6 +-
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c
> b/drivers/staging/media/atomisp/i2c/mt9m114.c
> index 8762124..a555aec 100644
> --- a/drivers/staging/media/atomisp/i2c/mt9m114.c
> +++ b/drivers/staging/media/atomisp/i2c/mt9m114.c
> @@ -444,12 +444,8 @@ static int mt9m114_set_suspend(struct v4l2_subdev *sd)
> static int mt9m114_init_common(struct v4l2_subdev *sd)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> - int ret;
>
> - ret = mt9m114_write_reg_array(client, mt9m114_common, PRE_POLLING);
> - if (ret)
> - return ret;
> - return ret;
> + return mt9m114_write_reg_array(client, mt9m114_common, PRE_POLLING);
> }
any use for "client" ?
re,
wh
>
> static int power_ctrl(struct v4l2_subdev *sd, bool flag)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: fix incorrect copy of pmkid data
Am 17.03.2017 00:21, schrieb Colin King:
> From: Colin Ian King
>
> The pmkid data is meant be be copied to the previous item in the
> pmkidlist, however the code is just copying the data to itself because
> the src index into pmkidlist is the same as the dst index into pmkidlist.
> Fix this with i + 1 instead of i.
>
> Detected by CoverityScan,CID#13339465 ("Overlapping buffer in memory copy")
>
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index a37896f..4034f40 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -1346,7 +1346,7 @@ static int del_pmksa(struct wiphy *wiphy, struct
> net_device *netdev,
> priv->pmkid_list.pmkidlist[i + 1].bssid,
> ETH_ALEN);
> memcpy(priv->pmkid_list.pmkidlist[i].pmkid,
> -priv->pmkid_list.pmkidlist[i].pmkid,
> +priv->pmkid_list.pmkidlist[i + 1].pmkid,
> PMKID_LEN);
> }
> priv->pmkid_list.numpmkid--;
perhaps we can also simplify the error handling:
that would reduce the indentlevel by one and effectivly remove the s32Error
variable.
if (i >= priv->pmkid_list.numpmkid || priv->pmkid_list.numpmkid <= 0)
return -EINVAL;
just my 2 cents.
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()
Am 20.03.2017 11:59, schrieb Daeseok Youn:
> If v4l2_subdev_call() gets the global frame interval values,
> it returned 0 and it could be checked whether numerator is zero or not.
>
> If the numerator is not zero, the fps could be calculated in this function.
> If not, it just returns 0.
>
> Signed-off-by: Daeseok Youn
> ---
> .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 22
> ++
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> index 8bdb224..6bdd19e 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct
> video_device *dev)
>
> static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device *asd)
> {
> - struct v4l2_subdev_frame_interval frame_interval;
> + struct v4l2_subdev_frame_interval fi;
> struct atomisp_device *isp = asd->isp;
> - unsigned short fps;
>
> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> - video, g_frame_interval, &frame_interval)) {
> - fps = 0;
> - } else {
> - if (frame_interval.interval.numerator)
> - fps = frame_interval.interval.denominator /
> - frame_interval.interval.numerator;
> - else
> - fps = 0;
> - }
> + unsigned short fps = 0;
> + int ret;
> +
> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
> +video, g_frame_interval, &fi);
> +
> + if (!ret && fi.interval.numerator)
> + fps = fi.interval.denominator / fi.interval.numerator;
> +
> return fps;
> }
do you need to check ret at all ? if an error occurs can fi.interval.numerator
be something else than 0 ?
if ret is an ERRNO it would be wise to return ret not fps, but this may require
changes at other places also.
re,
wh
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: atomisp: simplify if statement in atomisp_get_sensor_fps()
Am 20.03.2017 13:51, schrieb DaeSeok Youn:
> 2017-03-20 21:04 GMT+09:00 walter harms :
>>
>>
>> Am 20.03.2017 11:59, schrieb Daeseok Youn:
>>> If v4l2_subdev_call() gets the global frame interval values,
>>> it returned 0 and it could be checked whether numerator is zero or not.
>>>
>>> If the numerator is not zero, the fps could be calculated in this function.
>>> If not, it just returns 0.
>>>
>>> Signed-off-by: Daeseok Youn
>>> ---
>>> .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 22
>>> ++
>>> 1 file changed, 10 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> index 8bdb224..6bdd19e 100644
>>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>>> @@ -153,20 +153,18 @@ struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct
>>> video_device *dev)
>>>
>>> static unsigned short atomisp_get_sensor_fps(struct atomisp_sub_device
>>> *asd)
>>> {
>>> - struct v4l2_subdev_frame_interval frame_interval;
>>> + struct v4l2_subdev_frame_interval fi;
>>> struct atomisp_device *isp = asd->isp;
>>> - unsigned short fps;
>>>
>>> - if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>>> - video, g_frame_interval, &frame_interval)) {
>>> - fps = 0;
>>> - } else {
>>> - if (frame_interval.interval.numerator)
>>> - fps = frame_interval.interval.denominator /
>>> - frame_interval.interval.numerator;
>>> - else
>>> - fps = 0;
>>> - }
>>> + unsigned short fps = 0;
>>> + int ret;
>>> +
>>> + ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
>>> +video, g_frame_interval, &fi);
>>> +
>>> + if (!ret && fi.interval.numerator)
>>> + fps = fi.interval.denominator / fi.interval.numerator;
>>> +
>>> return fps;
>>> }
>>
>>
>>
>> do you need to check ret at all ? if an error occurs can
>> fi.interval.numerator
>> be something else than 0 ?
> the return value from the v4l2_subdev_call() function is zero when it
> is done without any error. and also I checked
> the ret value whether is 0 or not. if the ret is 0 then the value of
> numerator should be checked to avoid for dividing by 0.
>>
>> if ret is an ERRNO it would be wise to return ret not fps, but this may
>> require
>> changes at other places also.
> hmm.., yes, you are right. but I think it is ok because the
> atomisp_get_sensor_fps() function is needed to get fps value.
> (originally, zero or calculated fps value was returned.)
maybe its better to divide this in:
if (ret)
return 0; // error case
return (fi.interval.numerator>0)?fi.interval.denominator /
fi.interval.numerator:0;
So there is a chance that someone will a) understand and b) fix the error
return.
re,
wh
>
>>
>> re,
>> wh
>>
>>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: atomisp: use local variable to reduce the number of reference
Am 30.03.2017 08:25, schrieb Daeseok Youn:
> Define new local variable to reduce the number of reference.
> The new local variable is added to save the addess of dfs
> and used in atomisp_freq_scaling() function.
>
> Signed-off-by: Daeseok Youn
> ---
> .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 37
> --
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> index eebfccd..d76a95c 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
> @@ -251,6 +251,7 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
> {
> /* FIXME! Only use subdev[0] status yet */
> struct atomisp_sub_device *asd = &isp->asd[0];
> + const struct atomisp_dfs_config *dfs;
> unsigned int new_freq;
> struct atomisp_freq_scaling_rule curr_rules;
> int i, ret;
> @@ -268,20 +269,22 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
> ATOMISP_USE_YUVPP(asd))
> isp->dfs = &dfs_config_cht_soc;
>
> - if (isp->dfs->lowest_freq == 0 || isp->dfs->max_freq_at_vmin == 0 ||
> - isp->dfs->highest_freq == 0 || isp->dfs->dfs_table_size == 0 ||
> - !isp->dfs->dfs_table) {
> + dfs = isp->dfs;
> +
> + if (dfs->lowest_freq == 0 || dfs->max_freq_at_vmin == 0 ||
> + dfs->highest_freq == 0 || dfs->dfs_table_size == 0 ||
> + !dfs->dfs_table) {
> dev_err(isp->dev, "DFS configuration is invalid.\n");
> return -EINVAL;
> }
>
> if (mode == ATOMISP_DFS_MODE_LOW) {
> - new_freq = isp->dfs->lowest_freq;
> + new_freq = dfs->lowest_freq;
> goto done;
> }
>
> if (mode == ATOMISP_DFS_MODE_MAX) {
> - new_freq = isp->dfs->highest_freq;
> + new_freq = dfs->highest_freq;
> goto done;
> }
>
> @@ -307,26 +310,26 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
> }
>
> /* search for the target frequency by looping freq rules*/
> - for (i = 0; i < isp->dfs->dfs_table_size; i++) {
> - if (curr_rules.width != isp->dfs->dfs_table[i].width &&
> - isp->dfs->dfs_table[i].width != ISP_FREQ_RULE_ANY)
> + for (i = 0; i < dfs->dfs_table_size; i++) {
> + if (curr_rules.width != dfs->dfs_table[i].width &&
> + dfs->dfs_table[i].width != ISP_FREQ_RULE_ANY)
> continue;
> - if (curr_rules.height != isp->dfs->dfs_table[i].height &&
> - isp->dfs->dfs_table[i].height != ISP_FREQ_RULE_ANY)
> + if (curr_rules.height != dfs->dfs_table[i].height &&
> + dfs->dfs_table[i].height != ISP_FREQ_RULE_ANY)
> continue;
> - if (curr_rules.fps != isp->dfs->dfs_table[i].fps &&
> - isp->dfs->dfs_table[i].fps != ISP_FREQ_RULE_ANY)
> + if (curr_rules.fps != dfs->dfs_table[i].fps &&
> + dfs->dfs_table[i].fps != ISP_FREQ_RULE_ANY)
> continue;
> - if (curr_rules.run_mode != isp->dfs->dfs_table[i].run_mode &&
> - isp->dfs->dfs_table[i].run_mode != ISP_FREQ_RULE_ANY)
> + if (curr_rules.run_mode != dfs->dfs_table[i].run_mode &&
> + dfs->dfs_table[i].run_mode != ISP_FREQ_RULE_ANY)
> continue;
> break;
> }
>
> - if (i == isp->dfs->dfs_table_size)
> - new_freq = isp->dfs->max_freq_at_vmin;
> + if (i == dfs->dfs_table_size)
> + new_freq = dfs->max_freq_at_vmin;
> else
> - new_freq = isp->dfs->dfs_table[i].isp_freq;
> + new_freq = dfs->dfs_table[i].isp_freq;
>
you can eliminate the last block by setting
new_freq = dfs->max_freq_at_vmin;
for(i=0;) {
new_freq = dfs->dfs_table[i].isp_freq;
break;
}
unfortunately i have no good idea how to make the loop more readable.
re,
wh
> done:
> dev_dbg(isp->dev, "DFS target frequency=%d.\n", new_freq);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 8/11] staging: lustre: obdclass: Use kzalloc and kfree
hi Julia,
your patch seems fine.
I tried to understand the code and it seems that much of it
can be simplified by using already available functions.
I have added some comments but i am not sure what to make of it.
re,
wh
Am 01.05.2015 17:51, schrieb Julia Lawall:
> From: Julia Lawall
>
> Replace OBD_ALLOC, OBD_ALLOC_WAIT, OBD_ALLOC_PTR, and OBD_ALLOC_PTR_WAIT by
> kalloc/kcalloc, and OBD_FREE and OBD_FREE_PTR by kfree.
>
> A simplified version of the semantic patch that makes these changes is as
> follows: (http://coccinelle.lip6.fr/)
>
> //
> @@ expression ptr,size; @@
> - OBD_ALLOC(ptr,size)
> + ptr = kzalloc(size, GFP_NOFS)
>
> @@ expression ptr, size; @@
> - OBD_FREE(ptr, size);
> + kfree(ptr);
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> drivers/staging/lustre/lustre/obdclass/acl.c| 29 ++---
> drivers/staging/lustre/lustre/obdclass/capa.c |4
> drivers/staging/lustre/lustre/obdclass/cl_io.c | 13 +-
> drivers/staging/lustre/lustre/obdclass/cl_page.c|2
> drivers/staging/lustre/lustre/obdclass/class_obd.c |4
> drivers/staging/lustre/lustre/obdclass/genops.c | 40 +++
> drivers/staging/lustre/lustre/obdclass/llog.c | 24 ++--
> drivers/staging/lustre/lustre/obdclass/llog_obd.c |4
> drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 14 +-
> drivers/staging/lustre/lustre/obdclass/lu_object.c |6 -
> drivers/staging/lustre/lustre/obdclass/lustre_handles.c |2
> drivers/staging/lustre/lustre/obdclass/lustre_peer.c|6 -
> drivers/staging/lustre/lustre/obdclass/obd_config.c | 50 -
> drivers/staging/lustre/lustre/obdclass/obd_mount.c | 86
> +++-
> 14 files changed, 138 insertions(+), 146 deletions(-)
>
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> @@ -84,7 +84,7 @@ int lustre_process_log(struct super_bloc
> LASSERT(mgc);
> LASSERT(cfg);
>
> - OBD_ALLOC_PTR(bufs);
> + bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
> if (bufs == NULL)
> return -ENOMEM;
>
> @@ -97,7 +97,7 @@ int lustre_process_log(struct super_bloc
> rc = obd_process_config(mgc, sizeof(*lcfg), lcfg);
> lustre_cfg_free(lcfg);
>
> - OBD_FREE_PTR(bufs);
> + kfree(bufs);
>
> if (rc == -EINVAL)
> LCONSOLE_ERROR_MSG(0x15b, "%s: The configuration from log '%s'
> failed from the MGS (%d). Make sure this client and the MGS are running
> compatible versions of Lustre.\n",
> @@ -247,8 +247,8 @@ int lustre_start_mgc(struct super_block
> mutex_lock(&mgc_start_lock);
>
> len = strlen(LUSTRE_MGC_OBDNAME) + strlen(libcfs_nid2str(nid)) + 1;
> - OBD_ALLOC(mgcname, len);
> - OBD_ALLOC(niduuid, len + 2);
> + mgcname = kzalloc(len, GFP_NOFS);
> + niduuid = kzalloc(len + 2, GFP_NOFS);
> if (!mgcname || !niduuid) {
> rc = -ENOMEM;
> goto out_free;
this can be simplified by using
kasprintf(&mgcname,"%s%s", LUSTRE_MGC_OBDNAME, libcfs_nid2str(nid));
is guess the some is true for niduuid
> @@ -257,7 +257,7 @@ int lustre_start_mgc(struct super_block
>
> mgssec = lsi->lsi_lmd->lmd_mgssec ? lsi->lsi_lmd->lmd_mgssec : "";
>
> - OBD_ALLOC_PTR(data);
> + data = kzalloc(sizeof(*data), GFP_NOFS);
> if (data == NULL) {
> rc = -ENOMEM;
> goto out_free;
> @@ -375,7 +375,7 @@ int lustre_start_mgc(struct super_block
> lsi->lsi_lmd->lmd_mgs_failnodes = 1;
>
> /* Random uuid for MGC allows easier reconnects */
> - OBD_ALLOC_PTR(uuid);
> + uuid = kzalloc(sizeof(*uuid), GFP_NOFS);
> if (!uuid) {
> rc = -ENOMEM;
> goto out_free;
> @@ -388,7 +388,7 @@ int lustre_start_mgc(struct super_block
> rc = lustre_start_simple(mgcname, LUSTRE_MGC_NAME,
>(char *)uuid->uuid, LUSTRE_MGS_OBDNAME,
>niduuid, NULL, NULL);
> - OBD_FREE_PTR(uuid);
> + kfree(uuid);
> if (rc)
> goto out_free;
>
> @@ -465,11 +465,11 @@ out_free:
> mutex_unlock(&mgc_start_lock);
>
> if (data)
> - OBD_FREE_PTR(data);
> + kfree(data);
> if (mgcname)
> - OBD_FREE(mgcname, len);
> + kfree(mgcname);
> if (niduuid)
> - OBD_FREE(niduuid, len + 2);
> + kfree(niduuid);
> return rc;
> }
>
> @@ -513,7 +513,7 @@ static int lustre_stop_mgc(struct super_
> /* Save the obdname for cleaning the nid uuids, which are
> obdname_XX */
> len = strlen(obd->obd_name) + 6;
> - OBD_ALLOC(niduuid, len);
> + niduuid = kzalloc(len, GFP_NOFS);
> if (niduuid) {
> strcpy(
AW: [PATCH] staging: rtl8723bs: core: remove redundant zero'ing of counter variable k
Von: kernel-janitors-ow...@vger.kernel.org
im Auftrag von Colin King
Gesendet: Sonntag, 23. Februar 2020 16:28
An: Greg Kroah-Hartman; de...@driverdev.osuosl.org
Cc: kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
Betreff: [PATCH] staging: rtl8723bs: core: remove redundant zero'ing of counter
variable k
From: Colin Ian King
The zero'ing of counter variable k is redundant as it is never read
after breaking out of the while loop. Remove it.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King
---
drivers/staging/rtl8723bs/core/rtw_efuse.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_efuse.c
b/drivers/staging/rtl8723bs/core/rtw_efuse.c
index 3b8848182221..bdb6ff8aab7d 100644
--- a/drivers/staging/rtl8723bs/core/rtw_efuse.c
+++ b/drivers/staging/rtl8723bs/core/rtw_efuse.c
@@ -244,10 +244,8 @@ u16Address)
while (!(Bytetemp & 0x80)) {
Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
k++;
- if (k == 1000) {
- k = 0;
+ if (k == 1000)
break;
- }
IMHO this is confusing to read, i suggest:
for(k=0;k<1000;k++) {
Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
if ( Bytetemp & 0x80 )
break;
}
NTL is am wondering what will happen if k==1000
and Bytetemp is still invalid. Will rtw_read8() fail or
simply return invalid data ?
ym2c,
re,
wh
}
return rtw_read8(Adapter, EFUSE_CTRL);
} else
--
2.25.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
AW: [PATCH] staging: rtl8723bs: core: remove redundant zero'ing of counter variable k
Von: Dan Carpenter
Gesendet: Montag, 24. Februar 2020 12:27
An: Walter Harms
Cc: Colin King; Greg Kroah-Hartman; de...@driverdev.osuosl.org;
kernel-janit...@vger.kernel.org; linux-ker...@vger.kernel.org
Betreff: Re: [PATCH] staging: rtl8723bs: core: remove redundant zero'ing of
counter variable k
On Mon, Feb 24, 2020 at 11:07:55AM +0000, Walter Harms wrote:
> diff --git a/drivers/staging/rtl8723bs/core/rtw_efuse.c
> b/drivers/staging/rtl8723bs/core/rtw_efuse.c
> index 3b8848182221..bdb6ff8aab7d 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_efuse.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_efuse.c
> @@ -244,10 +244,8 @@ u16Address)
> while (!(Bytetemp & 0x80)) {
> Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
> k++;
> - if (k == 1000) {
> - k = 0;
> + if (k == 1000)
> break;
> - }
>
> IMHO this is confusing to read, i suggest:
>
> for(k=0;k<1000;k++) {
> Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
> if ( Bytetemp & 0x80 )
>break;
> }
>
The problem with the original code is that the variable is named "k"
instead of "retry". It should be:
do {
Bytetemp = rtw_read8(Adapter, EFUSE_CTRL+3);
} while (!(Bytetemp & 0x80)) && ++retry < 1000);
good point,
personally i try to avoid putting to much into braces, so i
would go for the additional if() but this is for the maintainer.
> NTL is am wondering what will happen if k==1000
> and Bytetemp is still invalid. Will rtw_read8() fail or
> simply return invalid data ?
Yeah. That was my thought reviewing this patch as well.
It should probably return 0xff on failure.
if (retry >= 1000)
return 0xff;
looks nice,
re,
wh
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 2/2] staging: r8188eu: overflow in rtw_p2p_get_go_device_address()
Am 03.02.2014 23:38, schrieb Dan Carpenter:
> The go_devadd_str[] array is two characters too small to hold the
> address so we corrupt memory.
>
> I've changed the user space API slightly and I don't have a way to test
> if this breaks anything. In the original code we truncated away the
> last digit of the address and the NUL terminator so it was already a bit
> broken.
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> index dec992569476..4ad80ae1067f 100644
> --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> @@ -3164,9 +3164,7 @@ static int rtw_p2p_get_go_device_address(struct
> net_device *dev,
> u8 *p2pie;
> uint p2pielen = 0, attr_contentlen = 0;
> u8 attr_content[100] = {0x00};
> -
> - u8 go_devadd_str[17 + 10] = {0x00};
> - /* +10 is for the str "go_devadd =", we have to clear it at
> wrqu->data.pointer */
> + u8 go_devadd_str[17 + 12] = {};
you are deleting the explanation for the magic numbers here,
- intentionally ?
NTL, it would be nice to have a full explanation like
10= space for "go_devadd ="
17= space for attr_content %.2X:%.2X:%.2X:%.2X:%.2X:%.2X
re,
wh
> /* Commented by Albert 20121209 */
> /* The input data is the GO's interface address which the
> application wants to know its device address. */
> @@ -3223,12 +3221,12 @@ static int rtw_p2p_get_go_device_address(struct
> net_device *dev,
> spin_unlock_bh(&pmlmepriv->scanned_queue.lock);
>
> if (!blnMatch)
> - sprintf(go_devadd_str, "\n\ndev_add = NULL");
> + snprintf(go_devadd_str, sizeof(go_devadd_str), "\n\ndev_add =
> NULL");
> else
> - sprintf(go_devadd_str, "\n\ndev_add
> =%.2X:%.2X:%.2X:%.2X:%.2X:%.2X",
> + snprintf(go_devadd_str, sizeof(go_devadd_str), "\n\ndev_add
> =%.2X:%.2X:%.2X:%.2X:%.2X:%.2X",
> attr_content[0], attr_content[1], attr_content[2],
> attr_content[3], attr_content[4], attr_content[5]);
>
> - if (copy_to_user(wrqu->data.pointer, go_devadd_str, 10 + 17))
> + if (copy_to_user(wrqu->data.pointer, go_devadd_str,
> sizeof(go_devadd_str)))
> return -EFAULT;
> return ret;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vme: Fix a small coding style issue in vme_user.c
Am 03.04.2014 00:24, schrieb Bojan Prtvar:
> The checkpatch.pl complains about missing blank line after declaration.
> This patch silence the warning.
>
> Signed-off-by: Bojan Prtvar
> ---
> This patch is for Eudyptula Challenge task 10
>
> drivers/staging/vme/devices/vme_user.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/vme/devices/vme_user.c
> b/drivers/staging/vme/devices/vme_user.c
> index 7927927..481192a 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -791,6 +791,7 @@ static int vme_user_probe(struct vme_dev *vdev)
> /* Add sysfs Entries */
> for (i = 0; i < VME_DEVS; i++) {
> int num;
> +
> switch (type[i]) {
> case MASTER_MINOR:
> sprintf(name, "bus/vme/m%%d");
beside style:
that sprint looks like as it could be replaced with a
name="bus/vme/m%%d"
any takers ?
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: vme: Simplfy string usage in vme_user_probe()
looks good to me
reviewed-by: wharms
@Bojan Prtvar:
please note that i am not the maintainer
Am 03.04.2014 18:56, schrieb Bojan Prtvar:
> We can avoid usage of sprintf() and magic-sized array with simple pointer
> assignment.
>
> Signed-off-by: Bojan Prtvar
> ---
> This parch should be applied on top of
> "staging: vme: Fix a small coding style issue in vme_user.c"
>
> Compile tested only.
>
> drivers/staging/vme/devices/vme_user.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/vme/devices/vme_user.c
> b/drivers/staging/vme/devices/vme_user.c
> index 481192a..5fa7f83 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -684,7 +684,7 @@ static int vme_user_match(struct vme_dev *vdev)
> static int vme_user_probe(struct vme_dev *vdev)
> {
> int i, err;
> - char name[12];
> + char *name;
>
> /* Save pointer to the bridge device */
> if (vme_user_bridge != NULL) {
> @@ -794,13 +794,13 @@ static int vme_user_probe(struct vme_dev *vdev)
>
> switch (type[i]) {
> case MASTER_MINOR:
> - sprintf(name, "bus/vme/m%%d");
> + name = "bus/vme/m%d";
> break;
> case CONTROL_MINOR:
> - sprintf(name, "bus/vme/ctl");
> + name = "bus/vme/ctl";
> break;
> case SLAVE_MINOR:
> - sprintf(name, "bus/vme/s%%d");
> + name = "bus/vme/s%d";
> break;
> default:
> err = -EINVAL;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: drivers: pcl812.c: fixed a coding style issue
Am 04.04.2014 12:05, schrieb Kumar Amit Mehta:
> Fixed a coding style issue. Reported by checkpatch.pl
>
> Signed-off-by: Kumar Amit Mehta
> ---
> drivers/staging/comedi/drivers/pcl812.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/comedi/drivers/pcl812.c
> b/drivers/staging/comedi/drivers/pcl812.c
> index 160eac8..5cc01fe 100644
> --- a/drivers/staging/comedi/drivers/pcl812.c
> +++ b/drivers/staging/comedi/drivers/pcl812.c
> @@ -811,8 +811,9 @@ static int pcl812_ai_cmd(struct comedi_device *dev,
> struct comedi_subdevice *s)
> devpriv->ai_dma = 0;
> break;
> }
> - } else
> + } else {
> devpriv->ai_dma = 0;
> + }
>
> devpriv->ai_act_scan = 0;
> devpriv->ai_poll_ptr = 0;
hi Kumar,
is that else needed at all ? perhaps it is possible to devpriv->ai_dma=0 before
the if ?
That would reduce code and give a better readability.
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 2/2] staging: lustre: integer overflow in obd_ioctl_is_invalid()
Am 24.04.2014 23:49, schrieb Dan Carpenter:
> The obd_ioctl_getdata() function caps "data->ioc_len" at
> OBD_MAX_IOCTL_BUFFER and then calls this obd_ioctl_is_invalid() to check
> that the other values inside data are valid.
>
> There are several lengths inside data but when they are added together
> they must not be larger than "data->ioc_len". The checks against
> "(data->ioc_inllen1 > (1<<30))" are supposed to ensure that the addition
> does not have an integer overflow. But "(1<<30) * 4" actually can
> overflow 32 bits so the checks are insufficient.
>
> I have changed it to "> OBD_MAX_IOCTL_BUFFER" instead.
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/lustre/lustre/include/lustre_lib.h
> b/drivers/staging/lustre/lustre/include/lustre_lib.h
> index 0368ca6..04f549e 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_lib.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_lib.h
> @@ -192,23 +192,23 @@ static inline int obd_ioctl_packlen(struct
> obd_ioctl_data *data)
>
> static inline int obd_ioctl_is_invalid(struct obd_ioctl_data *data)
> {
> - if (data->ioc_len > (1<<30)) {
> + if (data->ioc_len > OBD_MAX_IOCTL_BUFFER) {
> CERROR("OBD ioctl: ioc_len larger than 1<<30\n");
> return 1;
> }
I would suggest to adjust the errormsg also like:
CERROR("OBD ioctl: ioc_len larger than OBD_MAX_IOCTL_BUFFER\n");
otherwise future debuggers will be confused.
just my 2 cents
re,
wh
> - if (data->ioc_inllen1 > (1<<30)) {
> + if (data->ioc_inllen1 > OBD_MAX_IOCTL_BUFFER) {
> CERROR("OBD ioctl: ioc_inllen1 larger than 1<<30\n");
> return 1;
> }
> - if (data->ioc_inllen2 > (1<<30)) {
> + if (data->ioc_inllen2 > OBD_MAX_IOCTL_BUFFER) {
> CERROR("OBD ioctl: ioc_inllen2 larger than 1<<30\n");
> return 1;
> }
> - if (data->ioc_inllen3 > (1<<30)) {
> + if (data->ioc_inllen3 > OBD_MAX_IOCTL_BUFFER) {
> CERROR("OBD ioctl: ioc_inllen3 larger than 1<<30\n");
> return 1;
> }
> - if (data->ioc_inllen4 > (1<<30)) {
> + if (data->ioc_inllen4 > OBD_MAX_IOCTL_BUFFER) {
> CERROR("OBD ioctl: ioc_inllen4 larger than 1<<30\n");
> return 1;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch 2/2 v2] staging: lustre: integer overflow in obd_ioctl_is_invalid()
Am 28.04.2014 12:58, schrieb Dan Carpenter:
> The obd_ioctl_getdata() function caps "data->ioc_len" at
> OBD_MAX_IOCTL_BUFFER and then calls this obd_ioctl_is_invalid() to check
> that the other values inside data are valid.
>
> There are several lengths inside data but when they are added together
> they must not be larger than "data->ioc_len". The checks against
> "(data->ioc_inllen1 > (1<<30))" are supposed to ensure that the addition
> does not have an integer overflow. But "(1<<30) * 4" actually can
> overflow 32 bits, so the checks are insufficient.
>
> I have changed it to "> OBD_MAX_IOCTL_BUFFER" instead.
>
> Signed-off-by: Dan Carpenter
> ---
> v2: Updated the error messages as Walter Harms pointed out.
>
> diff --git a/drivers/staging/lustre/lustre/include/lustre_lib.h
> b/drivers/staging/lustre/lustre/include/lustre_lib.h
> index bdc9812..3c26bbd 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_lib.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_lib.h
> @@ -179,24 +179,25 @@ static inline int obd_ioctl_packlen(struct
> obd_ioctl_data *data)
>
> static inline int obd_ioctl_is_invalid(struct obd_ioctl_data *data)
> {
> - if (data->ioc_len > (1<<30)) {
> - CERROR("OBD ioctl: ioc_len larger than 1<<30\n");
> + if (data->ioc_len > OBD_MAX_IOCTL_BUFFER) {
> + CERROR("OBD ioctl: ioc_len larger than %d\n",
> +OBD_MAX_IOCTL_BUFFER);
> return 1;
> }
> - if (data->ioc_inllen1 > (1<<30)) {
> - CERROR("OBD ioctl: ioc_inllen1 larger than 1<<30\n");
> + if (data->ioc_inllen1 > OBD_MAX_IOCTL_BUFFER) {
> + CERROR("OBD ioctl: ioc_inllen1 larger than ioc_len\n");
> return 1;
> }
The error mention ioc_len the compare is OBD_MAX_IOCTL_BUFFER ?
Is that intentional ?
re,
wh
> - if (data->ioc_inllen2 > (1<<30)) {
> - CERROR("OBD ioctl: ioc_inllen2 larger than 1<<30\n");
> + if (data->ioc_inllen2 > OBD_MAX_IOCTL_BUFFER) {
> + CERROR("OBD ioctl: ioc_inllen2 larger than ioc_len\n");
> return 1;
> }
> - if (data->ioc_inllen3 > (1<<30)) {
> - CERROR("OBD ioctl: ioc_inllen3 larger than 1<<30\n");
> + if (data->ioc_inllen3 > OBD_MAX_IOCTL_BUFFER) {
> + CERROR("OBD ioctl: ioc_inllen3 larger than ioc_len\n");
> return 1;
> }
> - if (data->ioc_inllen4 > (1<<30)) {
> - CERROR("OBD ioctl: ioc_inllen4 larger than 1<<30\n");
> + if (data->ioc_inllen4 > OBD_MAX_IOCTL_BUFFER) {
> + CERROR("OBD ioctl: ioc_inllen4 larger than ioc_len\n");
> return 1;
> }
> if (data->ioc_inlbuf1 && !data->ioc_inllen1) {
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging/atomisp: putting NULs in the wrong place
Am 15.05.2017 12:01, schrieb Dan Carpenter:
> We're putting the NUL terminators one space beyond where they belong.
> This doesn't show up in testing because all but the callers put a NUL in
> the correct place themselves. LOL. It causes a static checker warning
> about buffer overflows.
>
> Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> Signed-off-by: Dan Carpenter
>
> diff --git
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/string_support.h
>
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/string_support.h
> index 74b5a1c7ac9a..c53241a7a281 100644
> ---
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/string_support.h
> +++
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/string_support.h
> @@ -117,7 +117,7 @@ STORAGE_CLASS_INLINE int strncpy_s(
>
> /* dest_str is big enough for the len */
> strncpy(dest_str, src_str, len);
> - dest_str[len+1] = '\0';
> + dest_str[len] = '\0';
> return 0;
> }
>
> @@ -157,7 +157,7 @@ STORAGE_CLASS_INLINE int strcpy_s(
>
> /* dest_str is big enough for the len */
> strncpy(dest_str, src_str, len);
> - dest_str[len+1] = '\0';
> + dest_str[len] = '\0';
> return 0;
> }
>
can this strcpy_s() replaced with strlcpy ?
re,
wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] atomisp: ensure that status values > 7 are reported as errors
Am 06.06.2017 18:30, schrieb Colin King:
> From: Colin Ian King
>
> The current code checks if a status value is greater than 7 and
> sets the status string as "ERROR" and then over writes the
> string based on the bottom 3 bits of the value. Instead, fix this by
> only checking on the bottom 3 bits of the value if the value is less
> than 8.
>
> Detected by CoverityScan, CID#1416607 ("Unused value")
>
> Signed-off-by: Colin Ian King
> ---
> .../css2400/runtime/debug/src/ia_css_debug.c | 141
> +++--
> 1 file changed, 72 insertions(+), 69 deletions(-)
>
> diff --git
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
>
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
> index bcc0d464084f..80d6fe29f30d 100644
> ---
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
> +++
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/debug/src/ia_css_debug.c
> @@ -765,28 +765,29 @@ static void
> debug_print_if_state(input_formatter_state_t *state, const char *id)
>
> val = state->fsm_sync_status;
>
> - if (val > 7)
> + if (val > 7) {
> fsm_sync_status_str = "ERROR";
> -
> - switch (val & 0x7) {
> - case 0:
> - fsm_sync_status_str = "idle";
> - break;
> - case 1:
> - fsm_sync_status_str = "request frame";
> - break;
> - case 2:
> - fsm_sync_status_str = "request lines";
> - break;
> - case 3:
> - fsm_sync_status_str = "request vectors";
> - break;
> - case 4:
> - fsm_sync_status_str = "send acknowledge";
> - break;
> - default:
> - fsm_sync_status_str = "unknown";
> - break;
> + } else {
> + switch (val & 0x7) {
> + case 0:
> + fsm_sync_status_str = "idle";
> + break;
> + case 1:
> + fsm_sync_status_str = "request frame";
> + break;
> + case 2:
> + fsm_sync_status_str = "request lines";
> + break;
> + case 3:
> + fsm_sync_status_str = "request vectors";
> + break;
> + case 4:
> + fsm_sync_status_str = "send acknowledge";
> + break;
> + default:
> + fsm_sync_status_str = "unknown";
> + break;
> + }
> }
>
> ia_css_debug_dtrace(2, "\t\t%-32s: (0x%X: %s)\n",
> @@ -799,34 +800,35 @@ static void
> debug_print_if_state(input_formatter_state_t *state, const char *id)
>
> val = state->fsm_crop_status;
>
> - if (val > 7)
> + if (val > 7) {
> fsm_crop_status_str = "ERROR";
> -
> - switch (val & 0x7) {
> - case 0:
> - fsm_crop_status_str = "idle";
> - break;
> - case 1:
> - fsm_crop_status_str = "wait line";
> - break;
> - case 2:
> - fsm_crop_status_str = "crop line";
> - break;
> - case 3:
> - fsm_crop_status_str = "crop pixel";
> - break;
> - case 4:
> - fsm_crop_status_str = "pass pixel";
> - break;
> - case 5:
> - fsm_crop_status_str = "pass line";
> - break;
> - case 6:
> - fsm_crop_status_str = "lost line";
> - break;
> - default:
> - fsm_crop_status_str = "unknown";
> - break;
> + } else {
> + switch (val & 0x7) {
> + case 0:
> + fsm_crop_status_str = "idle";
> + break;
> + case 1:
> + fsm_crop_status_str = "wait line";
> + break;
> + case 2:
> + fsm_crop_status_str = "crop line";
> + break;
> + case 3:
> + fsm_crop_status_str = "crop pixel";
> + break;
> + case 4:
> + fsm_crop_status_str = "pass pixel";
> + break;
> + case 5:
> + fsm_crop_status_str = "pass line";
> + break;
> + case 6:
> + fsm_crop_status_str = "lost line";
> + break;
> + default:
> + fsm_crop_status_str = "unknown";
> + break;
> + }
why not:
case 7:
fsm_crop_status_str = "unknown";
break;
default:
fsm_crop_status_str = "ERROR";
break;
looks more straigth to me..
same below.
less execptions, less errors
re,
wh
> }
> ia_css_debug_dtrace(2, "\t\t%-32s: (
Re: [PATCH] staging: dgnc: fix camelcase of SerialDriver and PrintDriver
You have send this patch before, right ?
then it is a good custom to have something like: [Patch V2] in the
subject line. In the comment you should write somethink like
v2: fix withspace damage
v1: fix issue
Otherwise none of the reviewer maintainer will see what was changes.
Sometimes patch run a few rounds before applied.
just my two cents
re,
wh
Am 22.03.2016 10:20, schrieb Daeseok Youn:
> fix the checkpatch.pl warning about CamelCase.
>
> Signed-off-by: Daeseok Youn
> ---
> drivers/staging/dgnc/dgnc_driver.h | 4 +-
> drivers/staging/dgnc/dgnc_tty.c| 118
> ++---
> 2 files changed, 61 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/staging/dgnc/dgnc_driver.h
> b/drivers/staging/dgnc/dgnc_driver.h
> index e4be81b..953c891 100644
> --- a/drivers/staging/dgnc/dgnc_driver.h
> +++ b/drivers/staging/dgnc/dgnc_driver.h
> @@ -202,9 +202,9 @@ struct dgnc_board {
>* to our channels.
>*/
>
> - struct tty_driver SerialDriver;
> + struct tty_driver serial_driver;
> charSerialName[200];
> - struct tty_driver PrintDriver;
> + struct tty_driver print_driver;
> charPrintName[200];
>
> booldgnc_Major_Serial_Registered;
> diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
> index bcd2bdf..081ac75 100644
> --- a/drivers/staging/dgnc/dgnc_tty.c
> +++ b/drivers/staging/dgnc/dgnc_tty.c
> @@ -178,20 +178,20 @@ int dgnc_tty_register(struct dgnc_board *brd)
> {
> int rc = 0;
>
> - brd->SerialDriver.magic = TTY_DRIVER_MAGIC;
> + brd->serial_driver.magic = TTY_DRIVER_MAGIC;
>
> snprintf(brd->SerialName, MAXTTYNAMELEN, "tty_dgnc_%d_", brd->boardnum);
>
> - brd->SerialDriver.name = brd->SerialName;
> - brd->SerialDriver.name_base = 0;
> - brd->SerialDriver.major = 0;
> - brd->SerialDriver.minor_start = 0;
> - brd->SerialDriver.num = brd->maxports;
> - brd->SerialDriver.type = TTY_DRIVER_TYPE_SERIAL;
> - brd->SerialDriver.subtype = SERIAL_TYPE_NORMAL;
> - brd->SerialDriver.init_termios = DgncDefaultTermios;
> - brd->SerialDriver.driver_name = DRVSTR;
> - brd->SerialDriver.flags = (TTY_DRIVER_REAL_RAW |
> + brd->serial_driver.name = brd->SerialName;
> + brd->serial_driver.name_base = 0;
> + brd->serial_driver.major = 0;
> + brd->serial_driver.minor_start = 0;
> + brd->serial_driver.num = brd->maxports;
> + brd->serial_driver.type = TTY_DRIVER_TYPE_SERIAL;
> + brd->serial_driver.subtype = SERIAL_TYPE_NORMAL;
> + brd->serial_driver.init_termios = DgncDefaultTermios;
> + brd->serial_driver.driver_name = DRVSTR;
> + brd->serial_driver.flags = (TTY_DRIVER_REAL_RAW |
> TTY_DRIVER_DYNAMIC_DEV |
> TTY_DRIVER_HARDWARE_BREAK);
>
> @@ -199,28 +199,28 @@ int dgnc_tty_register(struct dgnc_board *brd)
>* The kernel wants space to store pointers to
>* tty_struct's and termios's.
>*/
> - brd->SerialDriver.ttys = kcalloc(brd->maxports,
> - sizeof(*brd->SerialDriver.ttys),
> + brd->serial_driver.ttys = kcalloc(brd->maxports,
> + sizeof(*brd->serial_driver.ttys),
>GFP_KERNEL);
> - if (!brd->SerialDriver.ttys)
> + if (!brd->serial_driver.ttys)
> return -ENOMEM;
>
> - kref_init(&brd->SerialDriver.kref);
> - brd->SerialDriver.termios = kcalloc(brd->maxports,
> - sizeof(*brd->SerialDriver.termios),
> + kref_init(&brd->serial_driver.kref);
> + brd->serial_driver.termios = kcalloc(brd->maxports,
> + sizeof(*brd->serial_driver.termios),
> GFP_KERNEL);
> - if (!brd->SerialDriver.termios)
> + if (!brd->serial_driver.termios)
> return -ENOMEM;
>
> /*
>* Entry points for driver. Called by the kernel from
>* tty_io.c and n_tty.c.
>*/
> - tty_set_operations(&brd->SerialDriver, &dgnc_tty_ops);
> + tty_set_operations(&brd->serial_driver, &dgnc_tty_ops);
>
> if (!brd->dgnc_Major_Serial_Registered) {
> /* Register tty devices */
> - rc = tty_register_driver(&brd->SerialDriver);
> + rc = tty_register_driver(&brd->serial_driver);
> if (rc < 0) {
> dev_dbg(&brd->pdev->dev,
> "Can't register tty device (%d)\n", rc);
> @@ -234,19 +234,19 @@ int dgnc_tty_register(struct dgnc_board *brd)
>* again, separately so we don't get the LD confused about what major
>* we are when we get into the dgnc_tty_open() routine.
>*/
> - brd->PrintDriver.m
Re: [PATCH] staging: dgnc: replace dgnc_offset_table with bit shift.
Am 25.03.2016 12:33, schrieb Daeseok Youn:
> the dgnc_offset_table has a same value with (1 << port).
> So I tried to replace dgnc_offset_table array with 1 << port.
> And also there are redundant assignments(tmp and current_port)
> inside while loop for checking uart port, and remove them.
>
> Signed-off-by: Daeseok Youn
> ---
> Greg, This patch depends on previous patches.
> here are links(previous):
> 1. https://lkml.org/lkml/2016/3/24/661
> 2. https://lkml.org/lkml/2016/3/24/663
>
> if those patches are failed to merge, I will send them again after
> fixing them.
>
> thanks.
>
> drivers/staging/dgnc/dgnc_neo.c | 44
> +++--
> 1 file changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
> index d732e6e..8b6bc73 100644
> --- a/drivers/staging/dgnc/dgnc_neo.c
> +++ b/drivers/staging/dgnc/dgnc_neo.c
> @@ -77,9 +77,6 @@ struct board_ops dgnc_neo_ops = {
> .send_immediate_char = neo_send_immediate_char
> };
>
> -static uint dgnc_offset_table[8] = { 0x01, 0x02, 0x04, 0x08,
> - 0x10, 0x20, 0x40, 0x80 };
> -
> /*
> * This function allows calls to ensure that all outstanding
> * PCI writes have been completed, by doing a PCI read against
> @@ -923,9 +920,7 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
> struct dgnc_board *brd = voidbrd;
> struct channel_t *ch;
> int port = 0;
> - int type = 0;
> - int current_port;
> - u32 tmp;
> + int type;
> u32 uart_poll;
> unsigned long flags;
> unsigned long flags2;
> @@ -960,28 +955,29 @@ static irqreturn_t neo_intr(int irq, void *voidbrd)
>
> /* At this point, we have at least SOMETHING to service, dig further...
> */
>
> - current_port = 0;
> -
> /* Loop on each port */
> while ((uart_poll & 0xff) != 0) {
> - tmp = uart_poll;
> -
> - /* Check current port to see if it has interrupt pending */
> - if ((tmp & dgnc_offset_table[current_port]) != 0) {
> - port = current_port;
> - type = tmp >> (8 + (port * 3));
> - type &= 0x7;
> - } else {
> - current_port++;
> - continue;
> - }
> + int i;
>
> - /* Remove this port + type from uart_poll */
> - uart_poll &= ~(dgnc_offset_table[port]);
> + type = 0;
>
> - if (!type) {
> - /* If no type, just ignore it, and move onto next port
> */
> - continue;
> + for (i = port; i < MAXPORTS; i++) {
> + unsigned int offset_table = 0x1 << i;
> +
> + /* Check current port to see
> + * if it has interrupt pending
> + */
> + if ((uart_poll & offset_table) != 0) {
> + port = i;
> + type = uart_poll >> (8 + (port * 3));
> + type &= 0x7;
> +
> + /* Remove this port + type from uart_poll */
> + uart_poll &= ~(offset_table);
why not: art_poll &= ~ (0x1 << i);
then you can easy eliminate offset_table
btw: why do you need i ?
re,
wh
> + }
> +
> + if (type)
> + break;
> }
>
> /* Switch on type of interrupt we have */
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: wilc1000: fix mgmt_tx()
Am 10.02.2016 10:05, schrieb Dan Carpenter:
> There was a missing curly brace so this function returns failure instead
> of succeeding.
>
> Fixes: 06fb9336acdc ('staging: wilc1000: wilc_wfi_cfgoperations.c: replaces
> PRINT_ER with netdev_err')
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index bf264d3..97d1b80 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -1832,9 +1832,10 @@ static int mgmt_tx(struct wiphy *wiphy,
> return -EFAULT;
>
> mgmt_tx->buff = kmalloc(buf_len, GFP_KERNEL);
> - if (!mgmt_tx->buff)
> + if (!mgmt_tx->buff) {
> kfree(mgmt_tx);
> - return -EFAULT;
> + return -ENOMEM;
> + }
>
> memcpy(mgmt_tx->buff, buf, len);
> mgmt_tx->size = len;
perhaps this is a case for kmemdup() ?
mgmt_tx->buff = kmemdup(buf, buf_len or len ?, GFP_KERNEL);
sorry, i can not see what len from this patch.
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 09/12] staging: lustre: obdclass: Use !x to check for kzalloc failure
Am 20.06.2015 18:59, schrieb Julia Lawall:
> !x is more normal for kzalloc failure in the kernel.
>
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
>
> //
> @@
> expression x;
> statement S1, S2;
> @@
>
> x = kzalloc(...);
> if (
> - x == NULL
> + !x
> ) S1 else S2
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> drivers/staging/lustre/lustre/obdclass/class_obd.c |2 +-
> drivers/staging/lustre/lustre/obdclass/genops.c |6 +++---
> drivers/staging/lustre/lustre/obdclass/llog.c |6 +++---
> drivers/staging/lustre/lustre/obdclass/lprocfs_status.c |2 +-
> drivers/staging/lustre/lustre/obdclass/lustre_peer.c|2 +-
> drivers/staging/lustre/lustre/obdclass/obd_config.c | 10 +-
> drivers/staging/lustre/lustre/obdclass/obd_mount.c | 12 ++--
> 7 files changed, 20 insertions(+), 20 deletions(-)
>
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c
> @@ -85,7 +85,7 @@ int lustre_process_log(struct super_bloc
> LASSERT(cfg);
>
> bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
> - if (bufs == NULL)
> + if (!bufs)
> return -ENOMEM;
>
> /* mgc_process_config */
> @@ -258,7 +258,7 @@ int lustre_start_mgc(struct super_block
> mgssec = lsi->lsi_lmd->lmd_mgssec ? lsi->lsi_lmd->lmd_mgssec : "";
>
> data = kzalloc(sizeof(*data), GFP_NOFS);
> - if (data == NULL) {
> + if (!data) {
> rc = -ENOMEM;
> goto out_free;
> }
> @@ -885,7 +885,7 @@ static int lmd_parse_mgssec(struct lustr
> length = tail - ptr;
>
> lmd->lmd_mgssec = kzalloc(length + 1, GFP_NOFS);
> - if (lmd->lmd_mgssec == NULL)
> + if (!lmd->lmd_mgssec)
> return -ENOMEM;
>
> memcpy(lmd->lmd_mgssec, ptr, length);
looks like memdup()
> @@ -911,7 +911,7 @@ static int lmd_parse_string(char **handl
> length = tail - ptr;
>
> *handle = kzalloc(length + 1, GFP_NOFS);
> - if (*handle == NULL)
> + if (!*handle)
> return -ENOMEM;
>
> memcpy(*handle, ptr, length);
looks like memdup()
> @@ -941,7 +941,7 @@ static int lmd_parse_mgs(struct lustre_m
> oldlen = strlen(lmd->lmd_mgs) + 1;
>
> mgsnid = kzalloc(oldlen + length + 1, GFP_NOFS);
> - if (mgsnid == NULL)
> + if (!mgsnid)
> return -ENOMEM;
>
> if (lmd->lmd_mgs != NULL) {
> @@ -983,7 +983,7 @@ static int lmd_parse(char *options, stru
> lmd->lmd_magic = LMD_MAGIC;
>
> lmd->lmd_params = kzalloc(4096, GFP_NOFS);
> - if (lmd->lmd_params == NULL)
> + if (!lmd->lmd_params)
> return -ENOMEM;
> lmd->lmd_params[0] = '\0';
>
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/obd_config.c
> b/drivers/staging/lustre/lustre/obdclass/obd_config.c
> --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c
> +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c
> @@ -835,7 +835,7 @@ int class_add_profile(int proflen, char
> CDEBUG(D_CONFIG, "Add profile %s\n", prof);
>
> lprof = kzalloc(sizeof(*lprof), GFP_NOFS);
> - if (lprof == NULL)
> + if (!lprof)
> return -ENOMEM;
> INIT_LIST_HEAD(&lprof->lp_list);
>
> @@ -979,7 +979,7 @@ struct lustre_cfg *lustre_cfg_rename(str
> new_len = LUSTRE_CFG_BUFLEN(cfg, 1) + strlen(new_name) - name_len;
>
> new_param = kzalloc(new_len, GFP_NOFS);
> - if (new_param == NULL)
> + if (!new_param)
> return ERR_PTR(-ENOMEM);
>
> strcpy(new_param, new_name);
> @@ -987,7 +987,7 @@ struct lustre_cfg *lustre_cfg_rename(str
> strcat(new_param, value);
>
> bufs = kzalloc(sizeof(*bufs), GFP_NOFS);
> - if (bufs == NULL) {
> + if (!bufs) {
> kfree(new_param);
> return ERR_PTR(-ENOMEM);
> }
> @@ -1461,7 +1461,7 @@ int class_config_llog_handler(const stru
> inst_len = LUSTRE_CFG_BUFLEN(lcfg, 0) +
> sizeof(clli->cfg_instance) * 2 + 4;
> inst_name = kzalloc(inst_len, GFP_NOFS);
> - if (inst_name == NULL) {
> + if (!inst_name) {
> rc = -ENOMEM;
> goto out;
> }
> @@ -1639,7 +1639,7 @@ int class_config_dump_handler(const stru
> int rc = 0;
>
> outstr = kzalloc(256, GFP_NOFS);
> - if (outstr == NULL)
> + if (!outstr)
> return -ENOMEM;
>
> if (rec->lrh_type == OBD_CFG_REC) {
> diff -u -p a/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> b/drivers/staging/lustre/lustre/obdclass/lustre_peer.c
> --- a/drivers/staging/lustre/lu
Re: [patch] staging: comedi: das16: remove a duplicate condition
Am 29.07.2015 23:36, schrieb Dan Carpenter:
> We checked that "it->options[3]" was non-zero on the line before so
> there is no need to check again.
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/comedi/drivers/das16.c
> b/drivers/staging/comedi/drivers/das16.c
> index d7cf4b1..056bca9 100644
> --- a/drivers/staging/comedi/drivers/das16.c
> +++ b/drivers/staging/comedi/drivers/das16.c
> @@ -1032,8 +1032,7 @@ static int das16_attach(struct comedi_device *dev,
> struct comedi_devconfig *it)
>
> /* check that clock setting is valid */
> if (it->options[3]) {
> - if (it->options[3] != 0 &&
> - it->options[3] != 1 && it->options[3] != 10) {
> + if (it->options[3] != 1 && it->options[3] != 10) {
> dev_err(dev->class_dev,
> "Invalid option. Master clock must be set to 1
> or 10 (MHz)\n");
> return -EINVAL;
mmh, acording to the error messages 1 or 10 is allowed, obviously 0 is/was also
valid.
I would suggest to put that into one if () to make things more obvious.
It 0 is also valid a minor tweak to the error msg would be nice.
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: comedi: dt2811: fix a precedence bug
Am 21.06.2016 15:56, schrieb Ian Abbott:
> On 21/06/16 12:46, Dan Carpenter wrote:
>> Bitwise | has higher precedence than ?: so we need to add some
>> parenthesis for this to work as intended.
>>
>> Fixes: 7c9574090d30 ('staging: comedi: dt2811: simplify A/D reference
>> configuration')
>> Signed-off-by: Dan Carpenter
>>
>> diff --git a/drivers/staging/comedi/drivers/dt2811.c
>> b/drivers/staging/comedi/drivers/dt2811.c
>> index 904f6377..8bbd938 100644
>> --- a/drivers/staging/comedi/drivers/dt2811.c
>> +++ b/drivers/staging/comedi/drivers/dt2811.c
>> @@ -588,8 +588,8 @@ static int dt2811_attach(struct comedi_device
>> *dev, struct comedi_devconfig *it)
>> s = &dev->subdevices[0];
>> s->type= COMEDI_SUBD_AI;
>> s->subdev_flags= SDF_READABLE |
>> - (it->options[2] == 1) ? SDF_DIFF :
>> - (it->options[2] == 2) ? SDF_COMMON : SDF_GROUND;
>> + ((it->options[2] == 1) ? SDF_DIFF :
>> + (it->options[2] == 2) ? SDF_COMMON : SDF_GROUND);
>> s->n_chan= (it->options[2] == 1) ? 8 : 16;
>> s->maxdata= 0x0fff;
>> s->range_table= board->is_pgh ? &dt2811_pgh_ai_ranges
>>
>
perhaps we can improve readability with something like that (untested):
switch(it->options[2]) {
case 1:
s->subdev_flags= SDF_READABLE|SDF_DIFF;
break;
case 2:
s->subdev_flags= SDF_READABLE|SDF_COMMON;
break;
default:
s->subdev_flags=SDF_READABLE|SDF_GROUND;
}
so everyone has a chance to see what is going on.
just my 2 cents,
re,
wh
> Nice catch, thanks!
>
> Reviewed-by: Ian Abbott
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: wilc1000: NULL dereference on error
Am 16.07.2016 12:07, schrieb Dan Carpenter:
> We can't pass NULL pointers to destroy_workqueue().
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/wilc1000/host_interface.c
> b/drivers/staging/wilc1000/host_interface.c
> index 0b1760c..78f524f 100644
> --- a/drivers/staging/wilc1000/host_interface.c
> +++ b/drivers/staging/wilc1000/host_interface.c
> @@ -3363,7 +3363,7 @@ int wilc_init(struct net_device *dev, struct
> host_if_drv **hif_drv_handler)
> if (!hif_workqueue) {
> netdev_err(vif->ndev, "Failed to create workqueue\n");
> result = -ENOMEM;
> - goto _fail_mq_;
> + goto _fail_;
> }
>
maybe this is the point where return -ENOMEM; is the most simple solution ?
re,
wh
> setup_timer(&periodic_rssi, GetPeriodicRSSI,
> @@ -3391,7 +3391,6 @@ int wilc_init(struct net_device *dev, struct
> host_if_drv **hif_drv_handler)
>
> clients_count++;
>
> -_fail_mq_:
> destroy_workqueue(hif_workqueue);
> _fail_:
> return result;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: speakup: make function ser_to_dev static
Am 28.06.2017 15:13, schrieb Colin King:
> From: Colin Ian King
>
> The helper function ser_to_dev does not need to be in global scope, so
> make it static.
>
> Cleans up sparse warning:
> "warning: symbol 'ser_to_dev' was not declared. Should it be static?"
>
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/speakup/spk_ttyio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/speakup/spk_ttyio.c
> b/drivers/staging/speakup/spk_ttyio.c
> index 442f191a017e..ed8e96b06ead 100644
> --- a/drivers/staging/speakup/spk_ttyio.c
> +++ b/drivers/staging/speakup/spk_ttyio.c
> @@ -21,7 +21,7 @@ struct spk_ldisc_data {
> static struct spk_synth *spk_ttyio_synth;
> static struct tty_struct *speakup_tty;
>
> -int ser_to_dev(int ser, dev_t *dev_no)
> +static int ser_to_dev(int ser, dev_t *dev_no)
> {
> if (ser < 0 || ser > (255 - 64)) {
> pr_err("speakup: Invalid ser param. Must be between 0 and 191
> inclusive.\n");
Is there any remark, why the programmer decided to use (255 - 64) instead of
191 ?
re,
wh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: pi433: fix a precedence bug
Am 19.07.2017 11:51, schrieb Dan Carpenter:
> We had intended to do the mask first and then the shift. Shift has
> higher precedence so we need to add parenthesis.
>
> Fixes: 874bcba65f9a ("staging: pi433: New driver")
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index e391ce777bc7..5089449bf775 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -387,7 +387,7 @@ enum lnaGain rf69_get_lna_gain(struct spi_device *spi)
>
> currentValue = READ_REG(REG_LNA);
is currentValue used again later ?
iff not (IMHO) it would be more readable to use
currentValue &= MASK_LNA_CURRENT_GAIN
here.
and
currentValue >>3
inside the switch()
>
> - switch (currentValue & MASK_LNA_CURRENT_GAIN >> 3) // improvement:
> change 3 to define
> + switch ((currentValue & MASK_LNA_CURRENT_GAIN) >> 3) // improvement:
> change 3 to define
> {
> case LNA_GAIN_AUTO: return automatic;
> case LNA_GAIN_MAX: return max;
note:
and instead to add an artificial define for 3, it would be simpler to change
the LNA_GAIN_AUTO/MAX
to the "real" values and drop the >>3 for good.
just my 2 cents,
re,
wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: pi433: fix bugs in register abstraction of rf69 chip
Am 19.07.2017 20:18, schrieb Wolf Entwicklungen:
> Bugfixes for rf69_set_modulation, rf69_set_deviation, rf69_set_lna_gain and
> rf69_get_lna_gain
> The fixes are cross-checked with the datasheet of the rfm69cw
>
> Fixes: 874bcba65f9a ("staging: pi433: New driver")
> Signed-off-by: Marcus Wolf
>
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -101,7 +101,7 @@ int rf69_set_modulation(struct spi_device *spi, enum
> modulation modulation)
>
> currentValue = READ_REG(REG_DATAMODUL);
>
> - switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) // TODO
> improvement: change 3 to define
> + switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE)
> {
> case DATAMODUL_MODULATION_TYPE_OOK: return OOK;
> case DATAMODUL_MODULATION_TYPE_FSK: return FSK;
> @@ -203,7 +203,7 @@ int rf69_set_deviation(struct spi_device *spi, u32
> deviation)
> lsb = (f_reg&0xff);
>
> // check msb
> - if (msb & !FDEVMASB_MASK)
> + if (msb & ~FDEVMASB_MASK)
> {
> dev_dbg(&spi->dev, "set_deviation: err in calc of msb");
> INVALID_PARAM;
> @@ -366,13 +366,13 @@ int rf69_set_lna_gain(struct spi_device *spi, enum
> lnaGain lnaGain)
> #endif
>
> switch(lnaGain) {
> - case automatic: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) & LNA_GAIN_AUTO) );
> - case max:return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) & LNA_GAIN_MAX) );
> - case maxMinus6: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_6) );
> - case maxMinus12: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_12) );
> - case maxMinus24: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_24) );
> - case maxMinus36: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_36) );
> - case maxMinus48: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_48) );
> + case automatic: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) | LNA_GAIN_AUTO) );
> + case max:return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) | LNA_GAIN_MAX) );
> + case maxMinus6: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_6) );
> + case maxMinus12: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_12) );
> + case maxMinus24: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_24) );
> + case maxMinus36: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_36) );
> + case maxMinus48: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_48) );
> default: INVALID_PARAM;
> }
> }
looks resonable,
some nitpicking: perhaps you can can improve readability by using a helper like:
static int setstbit(int arg)
{
int get=READ_REG(REG_LNA) & ~MASK_LNA_GAIN;
return WRITE_REG( get| arg);
}
so this switch would be reduced to ...
case maxMinus6: return setstbit(LNA_GAIN_MAX_MINUS_6);
re,
wh
> @@ -387,7 +387,7 @@ enum lnaGain rf69_get_lna_gain(struct spi_device *spi)
>
> currentValue = READ_REG(REG_LNA);
>
> - switch (currentValue & MASK_LNA_CURRENT_GAIN >> 3) // improvement:
> change 3 to define
> + switch (currentValue & MASK_LNA_GAIN)
> {
> case LNA_GAIN_AUTO: return automatic;
> case LNA_GAIN_MAX: return max;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: pi433: fix bugs in register abstraction of rf69 chip
Am 20.07.2017 18:08, schrieb Marcus Wolf:
> Hi Walter,
>
> since the construction
>
> WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) & ~MASK_LNA_GAIN) |
> LNA_GAIN_MAX_MINUS_6) )
> aka
> WRITE_REG(regname, ( (READ_REG(regname) & ~regmask ) | vale
>) )
>
> is used nearly everywhere, I think, about using a more gneric macro like
>
> #define READ_MODIFY_WRITE(reg, mask, value) /
> WRITE_REG(reg, ((READ_REG(reg) & ~mask) | vale ))
>
>
> What do you think about it?
Yep, good idea.
personally i prefer functions because for a linux kernel the scope of
DEFINE is a bit large.
Small functions tend to be inlined by the compiler, so there is no speed
disadvatage. But this is a matter of taste and the maintainer has to maintain
whatever happens.
just my 2 cents
re,
wh
>
> Cheers,
>
> Marcus
>
>> walter harms hat am 20. Juli 2017 um 14:22 geschrieben:
>>
>>
>>
>>
>> Am 19.07.2017 20:18, schrieb Wolf Entwicklungen:
>>> Bugfixes for rf69_set_modulation, rf69_set_deviation, rf69_set_lna_gain and
>>> rf69_get_lna_gain
>>> The fixes are cross-checked with the datasheet of the rfm69cw
>>>
>>> Fixes: 874bcba65f9a ("staging: pi433: New driver")
>>> Signed-off-by: Marcus Wolf
>>>
>>> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
>>> --- a/drivers/staging/pi433/rf69.c
>>> +++ b/drivers/staging/pi433/rf69.c
>>> @@ -101,7 +101,7 @@ int rf69_set_modulation(struct spi_device *spi, enum
>>> modulation modulation)
>>>
>>> currentValue = READ_REG(REG_DATAMODUL);
>>>
>>> - switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) // TODO
>>> improvement: change 3 to define
>>> + switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE)
>>> {
>>> case DATAMODUL_MODULATION_TYPE_OOK: return OOK;
>>> case DATAMODUL_MODULATION_TYPE_FSK: return FSK;
>>> @@ -203,7 +203,7 @@ int rf69_set_deviation(struct spi_device *spi, u32
>>> deviation)
>>> lsb = (f_reg&0xff);
>>>
>>> // check msb
>>> - if (msb & !FDEVMASB_MASK)
>>> + if (msb & ~FDEVMASB_MASK)
>>> {
>>> dev_dbg(&spi->dev, "set_deviation: err in calc of msb");
>>> INVALID_PARAM;
>>> @@ -366,13 +366,13 @@ int rf69_set_lna_gain(struct spi_device *spi, enum
>>> lnaGain lnaGain)
>>> #endif
>>>
>>> switch(lnaGain) {
>>> - case automatic: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) & LNA_GAIN_AUTO) );
>>> - case max: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) & ~MASK_LNA_GAIN)
>>> & LNA_GAIN_MAX) );
>>> - case maxMinus6: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_6) );
>>> - case maxMinus12: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_12) );
>>> - case maxMinus24: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_24) );
>>> - case maxMinus36: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_36) );
>>> - case maxMinus48: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) & LNA_GAIN_MAX_MINUS_48) );
>>> + case automatic: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) | LNA_GAIN_AUTO) );
>>> + case max: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) & ~MASK_LNA_GAIN)
>>> | LNA_GAIN_MAX) );
>>> + case maxMinus6: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_6) );
>>> + case maxMinus12: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_12) );
>>> + case maxMinus24: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_24) );
>>> + case maxMinus36: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_36) );
>>> + case maxMinus48: return WRITE_REG(REG_LNA, ( (READ_REG(REG_LNA) &
>>> ~MASK_LNA_GAIN) | LNA_GAIN_MAX_MINUS_48) );
>>> default: INVALID_PARAM;
>>> }
>>> }
>>
>>
>> looks resonable,
>> some nitpicking: perhaps you can can improve readability by using a helper
>> like:
>>
>> static int setstbit(int arg)
>> {
>> int get=READ_REG(REG_LNA) & ~MAS
Re: [PATCH] staging: rtl8192u: fix incorrect mask when calculating TxPowerLevelCCK
Am 05.09.2017 18:32, schrieb Colin King:
> From: Colin Ian King
>
> The mask of 0xff and right shift of 8 bits on ret always results in
> a value of 0 for TxPowerLevelCCK. I believe this should be a mask of
> 0xff00, however I do not have the hardware at hand to test this out,
> so there is a distinct possibility I may be wrong on this.
>
> Detected by CoverityScan CID#1357110 ("Operands don't affect result")
>
> Signed-off-by: Colin Ian King
> ---
> drivers/staging/rtl8192u/r8192U_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c
> b/drivers/staging/rtl8192u/r8192U_core.c
> index 46b3f19e0878..ecc887636173 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -2510,7 +2510,7 @@ static int rtl8192_read_eeprom_info(struct net_device
> *dev)
> ret = eprom_read(dev, (EEPROM_TxPwIndex_CCK >>
> 1));
> if (ret < 0)
> return ret;
> - priv->EEPROMTxPowerLevelCCK = ((u16)ret & 0xff)
> >> 8;
> + priv->EEPROMTxPowerLevelCCK = ((u16)ret &
> 0xff00) >> 8;
Is there any need for the mask ?
(u16) will already reduce ret to 16 bit, the >>8 will shift the lower bits into
nirwana
re,
wh
> } else
> priv->EEPROMTxPowerLevelCCK = 0x10;
> RT_TRACE(COMP_EPROM, "CCK Tx Power Levl: 0x%02x\n",
> priv->EEPROMTxPowerLevelCCK);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: iio: tsl2x7x: clean up limit checks
Am 08.09.2017 12:53, schrieb Dan Carpenter:
> The background of this code is that we can either use the default
> tables or load our own table with sysfs. The default tables are three
> element arrays of struct tsl2x7x_lux. If we load the table with sysfs
> then we can have as many as nine elements. Which ever way we do it, the
> last element is always zeroed out.
>
> The most interesting part of this patch is in the
> in_illuminance0_lux_table_show() function. We were using the wrong
> limit, "TSL2X7X_MAX_LUX_TABLE_SIZE * 3", when it should have been just
> "TSL2X7X_MAX_LUX_TABLE_SIZE". This creates a static checker warning
> that we are going of of bounds. However, since the last element is
> always zeroed out, that means we hit the break statement and the code
> works correctly despite the wrong limit check.
>
> I made several related readability changes. The most notable that I
> changed the MAX_DEFAULT_TABLE_BYTES define which was:
>
> #define MAX_DEFAULT_TABLE_BYTES (sizeof(int) * TSL2X7X_MAX_LUX_TABLE_SIZE)
>
> I renamed the define to TSL2X7X_DEFAULT_TABLE_BYTES because it's not the
> max size, it's the only size. Also the size should really be expressed
> as sizeof(struct tsl2x7x_lux) * 3. In other words, 12 * 3 instead of
> 4 * 9. It's 36 bytes either way, so this doesn't change the behavior.
>
> Finally, I created the TSL2X7X_DEF_LUX_TABLE_SZ define instead of using
> the magic number 3. I declared the default tables using that define
> to hopefully signal to future programmers that if they want to use a
> different size they have to update all the related code.
>
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/iio/light/tsl2x7x.h
> b/drivers/staging/iio/light/tsl2x7x.h
> index ecae92211216..a216c6943a84 100644
> --- a/drivers/staging/iio/light/tsl2x7x.h
> +++ b/drivers/staging/iio/light/tsl2x7x.h
> @@ -23,10 +23,6 @@
> #define __TSL2X7X_H
> #include
>
> -/* Max number of segments allowable in LUX table */
> -#define TSL2X7X_MAX_LUX_TABLE_SIZE 9
> -#define MAX_DEFAULT_TABLE_BYTES (sizeof(int) * TSL2X7X_MAX_LUX_TABLE_SIZE)
> -
> struct iio_dev;
>
> struct tsl2x7x_lux {
> @@ -35,6 +31,13 @@ struct tsl2x7x_lux {
> unsigned int ch1;
> };
>
> +/* Max number of segments allowable in LUX table */
> +#define TSL2X7X_MAX_LUX_TABLE_SIZE 9
> +/* The default LUX tables all have 3 elements. */
> +#define TSL2X7X_DEF_LUX_TABLE_SZ 3
> +#define TSL2X7X_DEFAULT_TABLE_BYTES (sizeof(struct tsl2x7x_lux) * \
> + TSL2X7X_DEF_LUX_TABLE_SZ)
> +
> /**
> * struct tsl2x7x_default_settings - power on defaults unless
> * overridden by platform data.
> diff --git a/drivers/staging/iio/light/tsl2x7x.c
> b/drivers/staging/iio/light/tsl2x7x.c
> index 786e93f16ce9..b3a9efecf536 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -192,25 +192,25 @@ struct tsl2X7X_chip {
> };
>
> /* Different devices require different coefficents */
> -static const struct tsl2x7x_lux tsl2x71_lux_table[] = {
> +static const struct tsl2x7x_lux tsl2x71_lux_table[TSL2X7X_DEF_LUX_TABLE_SZ]
> = {
> { 14461, 611, 1211 },
> { 18540, 352,623 },
> { 0, 0, 0 },
> };
>
> -static const struct tsl2x7x_lux tmd2x71_lux_table[] = {
> +static const struct tsl2x7x_lux tmd2x71_lux_table[TSL2X7X_DEF_LUX_TABLE_SZ]
> = {
> { 11635, 115,256 },
> { 15536,87,179 },
> { 0, 0, 0 },
> };
>
> -static const struct tsl2x7x_lux tsl2x72_lux_table[] = {
> +static const struct tsl2x7x_lux tsl2x72_lux_table[TSL2X7X_DEF_LUX_TABLE_SZ]
> = {
> { 14013, 466, 917 },
> { 18222, 310, 552 },
> { 0, 0, 0 },
> };
>
> -static const struct tsl2x7x_lux tmd2x72_lux_table[] = {
> +static const struct tsl2x7x_lux tmd2x72_lux_table[TSL2X7X_DEF_LUX_TABLE_SZ]
> = {
> { 13218, 130, 262 },
> { 17592, 92,169 },
> { 0, 0, 0 },
> @@ -530,7 +530,7 @@ static void tsl2x7x_defaults(struct tsl2X7X_chip *chip)
> else
> memcpy(chip->tsl2x7x_device_lux,
> (struct tsl2x7x_lux *)tsl2x7x_default_lux_table_group[chip->id],
> - MAX_DEFAULT_TABLE_BYTES);
> + TSL2X7X_DEFAULT_TABLE_BYTES);
> }
>
> /**
> @@ -1113,7 +1113,7 @@ static ssize_t in_illuminance0_lux_table_show(struct
> device *dev,
> int i = 0;
> int offset = 0;
>
> - while (i < (TSL2X7X_MAX_LUX_TABLE_SIZE * 3)) {
> + while (i < TSL2X7X_MAX_LUX_TABLE_SIZE) {
> offset += snprintf(buf + offset, PAGE_SIZE, "%u,%u,%u,",
> chip->tsl2x7x_device_lux[i].ratio,
> chip->tsl2x7x_device_lux[i].ch0,
Is that TSL2X7X_MAX_LUX_TABLE_SIZE needed at all ? because:
if (chip->tsl2x7x_device_lux[i].ratio == 0) ... break;
So its
Re: [patch] Staging: vt6655-6: potential NULL dereference in hostap_disable_hostapd()
Am 07.11.2013 08:55, schrieb Dan Carpenter:
> We fixed this to use free_netdev() instead of kfree() but unfortunately
> free_netdev() doesn't accept NULL pointers. Smatch complains about
> this, it's not something I discovered through testing.
>
> Fixes: 3030d40b5036 ('staging: vt6655: use free_netdev instead of kfree')
> Fixes: 0a438d5b381e ('staging: vt6656: use free_netdev instead of kfree')
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/staging/vt6655/hostap.c b/drivers/staging/vt6655/hostap.c
> index aab0012..ab8b2ba 100644
> --- a/drivers/staging/vt6655/hostap.c
> +++ b/drivers/staging/vt6655/hostap.c
> @@ -143,7 +143,8 @@ static int hostap_disable_hostapd(PSDevice pDevice, int
> rtnl_locked)
> DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "%s: Netdevice %s
> unregistered\n",
> pDevice->dev->name, pDevice->apdev->name);
> }
> - free_netdev(pDevice->apdev);
> + if (pDevice->apdev)
> + free_netdev(pDevice->apdev);
perhaps the better way is to fix free_netdev() to make it behave like kfree()
and friends.
just my 2 cents,
wh
> pDevice->apdev = NULL;
> pDevice->bEnable8021x = false;
> pDevice->bEnableHostWEP = false;
> diff --git a/drivers/staging/vt6656/hostap.c b/drivers/staging/vt6656/hostap.c
> index ae1676d..67ba48b 100644
> --- a/drivers/staging/vt6656/hostap.c
> +++ b/drivers/staging/vt6656/hostap.c
> @@ -133,7 +133,8 @@ static int hostap_disable_hostapd(struct vnt_private
> *pDevice, int rtnl_locked)
> DBG_PRT(MSG_LEVEL_DEBUG, KERN_INFO "%s: Netdevice %s
> unregistered\n",
> pDevice->dev->name, pDevice->apdev->name);
> }
> - free_netdev(pDevice->apdev);
> + if (pDevice->apdev)
> + free_netdev(pDevice->apdev);
> pDevice->apdev = NULL;
> pDevice->bEnable8021x = false;
> pDevice->bEnableHostWEP = false;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [patch] staging: r8821ae: a couple macro expansion bugs
Am 28.01.2014 15:00, schrieb Dan Carpenter: > These macros need parentheses, otherwise it causes a macro expansion bug > when they are used like this: > > ch->flags &= ~IEEE80211_CHAN_NO_IBSS; > > This was found using Smatch: > drivers/staging/rtl8821ae/regd.c:200 _rtl_reg_apply_beaconing_flags() > warn: the 'IEEE80211_CHAN_NO_IBSS' macro might need parens > > Signed-off-by: Dan Carpenter > > diff --git a/drivers/staging/rtl8821ae/regd.h > b/drivers/staging/rtl8821ae/regd.h > index abc60ab8165c..dceb3f18200b 100644 > --- a/drivers/staging/rtl8821ae/regd.h > +++ b/drivers/staging/rtl8821ae/regd.h > @@ -30,8 +30,8 @@ > #ifndef __RTL_REGD_H__ > #define __RTL_REGD_H__ > > -#define IEEE80211_CHAN_NO_IBSS 1<<2 > -#define IEEE80211_CHAN_PASSIVE_SCAN 1<<1 > +#define IEEE80211_CHAN_NO_IBSS (1 << 2) > +#define IEEE80211_CHAN_PASSIVE_SCAN (1 << 1) > #define WIPHY_FLAG_CUSTOM_REGULATORY BIT(0) > #define WIPHY_FLAG_STRICT_REGULATORY BIT(1) > #define WIPHY_FLAG_DISABLE_BEACON_HINTS BIT(2) > -- just one minor hint ... could we settle for a common semantic here either 1<<2 or BIT(2) ? just my 2 cents, re, wh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/5] staging: lustre: lnet: socklnd: Clean up memset(...)
Am 18.05.2014 19:27, schrieb Joe Perches: > On Sun, 2014-05-18 at 18:19 +0100, Masaru Nomura wrote: >> Remove prohibited space and fix line over 80 characters of >> memset(...) to meet kernel coding style. > [] >> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c >> b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c > [] >> @@ -113,7 +113,7 @@ ksocknal_create_peer(ksock_peer_t **peerp, lnet_ni_t >> *ni, lnet_process_id_t id) >> if (peer == NULL) >> return -ENOMEM; >> >> -memset (peer, 0, sizeof (*peer)); /* NULL pointers/clear flags >> etc */ >> +memset(peer, 0, sizeof(*peer)); /* NULL pointers/clear flags etc */ > > It looks like this memset is unnecessary > as it's already zeroed by LIBCFS_ALLOC-> > LIBCFS_ALLOC_GFP->LIBCFS_ALLOC_POST->memset. > > It seems as if all these ALLOC macros could > use quite a bit of cleaning/sorting out. > for a start, some kind soul could replace the malloc()/memset() with zalloc(). re, wh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/8] staging: r8712u: Remove useless return variables
Am 20.05.2014 12:33, schrieb Peter Senna Tschudin:
> This patch remove variables that are initialized with a constant,
> are never updated, and are only used as parameter of return.
> Return the constant instead of using a variable.
>
> Verified by compilation only.
>
> The coccinelle script that find and fixes this issue is:
> //
> @@
> type T;
> constant C;
> identifier ret;
> @@
> - T ret = C;
> ... when != ret
> - return ret;
> + return C;
> //
>
> Signed-off-by: Peter Senna Tschudin
>
> ---
> drivers/staging/rtl8712/ieee80211.c|8 ++--
> drivers/staging/rtl8712/rtl8712_cmd.c |3 -
> drivers/staging/rtl8712/rtl8712_recv.c |7 +---
> drivers/staging/rtl8712/rtl871x_mlme.c |3 -
> drivers/staging/rtl8712/rtl871x_mp.c |3 -
> drivers/staging/rtl8712/rtl871x_mp_ioctl.c | 49
> +
> drivers/staging/rtl8712/rtl871x_recv.c |4 --
> 7 files changed, 26 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/ieee80211.c
> b/drivers/staging/rtl8712/ieee80211.c
> index 57fef70..fe9459e 100644
> --- a/drivers/staging/rtl8712/ieee80211.c
> +++ b/drivers/staging/rtl8712/ieee80211.c
> @@ -289,7 +289,7 @@ static int r8712_get_wpa2_cipher_suite(u8 *s)
> int r8712_parse_wpa_ie(u8 *wpa_ie, int wpa_ie_len, int *group_cipher,
>int *pairwise_cipher)
> {
> - int i, ret = _SUCCESS;
> + int i;
> int left, count;
> u8 *pos;
>
> @@ -324,13 +324,13 @@ int r8712_parse_wpa_ie(u8 *wpa_ie, int wpa_ie_len, int
> *group_cipher,
> }
> } else if (left == 1)
> return _FAIL;
> - return ret;
> + return _SUCCESS;
> }
>
> int r8712_parse_wpa2_ie(u8 *rsn_ie, int rsn_ie_len, int *group_cipher,
> int *pairwise_cipher)
> {
> - int i, ret = _SUCCESS;
> + int i;
> int left, count;
> u8 *pos;
>
> @@ -364,7 +364,7 @@ int r8712_parse_wpa2_ie(u8 *rsn_ie, int rsn_ie_len, int
> *group_cipher,
> }
> } else if (left == 1)
> return _FAIL;
> - return ret;
> + return _SUCCESS;
> }
>
> int r8712_get_sec_ie(u8 *in_ie, uint in_len, u8 *rsn_ie, u16 *rsn_len,
> diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c
> b/drivers/staging/rtl8712/rtl8712_cmd.c
> index 1a4b7a6..8ca7d7e 100644
> --- a/drivers/staging/rtl8712/rtl8712_cmd.c
> +++ b/drivers/staging/rtl8712/rtl8712_cmd.c
> @@ -290,8 +290,7 @@ static struct cmd_obj *cmd_hdl_filter(struct _adapter
> *padapter,
>
> static u8 check_cmd_fifo(struct _adapter *padapter, uint sz)
> {
> - u8 res = _SUCCESS;
> - return res;
> + return _SUCCESS;
> }
>
Is this function still needed ?
> u8 r8712_fw_cmd(struct _adapter *pAdapter, u32 cmd)
> diff --git a/drivers/staging/rtl8712/rtl8712_recv.c
> b/drivers/staging/rtl8712/rtl8712_recv.c
> index 0723b2f..667398a 100644
> --- a/drivers/staging/rtl8712/rtl8712_recv.c
> +++ b/drivers/staging/rtl8712/rtl8712_recv.c
> @@ -123,8 +123,6 @@ void r8712_free_recv_priv(struct recv_priv *precvpriv)
>
> int r8712_init_recvbuf(struct _adapter *padapter, struct recv_buf *precvbuf)
> {
> - int res = _SUCCESS;
> -
> precvbuf->transfer_len = 0;
> precvbuf->len = 0;
> precvbuf->ref_cnt = 0;
> @@ -134,7 +132,7 @@ int r8712_init_recvbuf(struct _adapter *padapter, struct
> recv_buf *precvbuf)
> precvbuf->ptail = precvbuf->pbuf;
> precvbuf->pend = precvbuf->pdata + MAX_RECVBUF_SZ;
> }
> - return res;
> + return _SUCCESS;
> }
>
> int r8712_free_recvframe(union recv_frame *precvframe,
> @@ -347,7 +345,6 @@ static int amsdu_to_msdu(struct _adapter *padapter, union
> recv_frame *prframe)
> _pkt *sub_skb, *subframes[MAX_SUBFRAME_COUNT];
> struct recv_priv *precvpriv = &padapter->recvpriv;
> struct __queue *pfree_recv_queue = &(precvpriv->free_recv_queue);
> - int ret = _SUCCESS;
>
> nr_subframes = 0;
> pattrib = &prframe->u.hdr.attrib;
> @@ -435,7 +432,7 @@ static int amsdu_to_msdu(struct _adapter *padapter, union
> recv_frame *prframe)
> exit:
> prframe->u.hdr.len = 0;
> r8712_free_recvframe(prframe, pfree_recv_queue);
> - return ret;
> + return _SUCCESS;
> }
>
> void r8712_rxcmd_event_hdl(struct _adapter *padapter, void *prxcmdbuf)
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
> b/drivers/staging/rtl8712/rtl871x_mlme.c
> index 3ea99ae..05d6e65 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -1211,7 +1211,6 @@ sint r8712_set_auth(struct _adapter *adapter,
> struct cmd_priv *pcmdpriv = &adapter->cmdpriv;
> struct cmd_obj *pcmd;
> struct setauth_parm *psetauthparm;
> - sint ret = _SUCCESS;
>
> pcmd = (struct cmd_obj *)_malloc(sizeof(struct cmd_obj));
> if (pcmd == NULL)
> @@ -1232,7 +1231,7 @@ sint r8712_set_auth(struct _adapter *adapter,
> pcmd->rspsz = 0
Re: [PATCH 3/8] drivers/staging: Remove useless return variables
Am 20.05.2014 12:33, schrieb Peter Senna Tschudin:
> This patch remove variables that are initialized with a constant,
> are never updated, and are only used as parameter of return.
> Return the constant instead of using a variable.
>
> Verified by compilation only.
>
> The coccinelle script that find and fixes this issue is:
> //
> @@
> type T;
> constant C;
> identifier ret;
> @@
> - T ret = C;
> ... when != ret
> - return ret;
> + return C;
> //
>
> Signed-off-by: Peter Senna Tschudin
>
> ---
> drivers/staging/bcm/Bcmchar.c |3 +--
> drivers/staging/bcm/InterfaceIdleMode.c |3 +--
> drivers/staging/bcm/PHSModule.c |9 +++--
> drivers/staging/gdm72xx/gdm_wimax.c |3 +--
> drivers/staging/rtl8192e/rtl8192e/r8192E_phy.c|3 +--
> drivers/staging/rtl8192e/rtllib_rx.c |3 +--
> drivers/staging/rtl8192e/rtllib_softmac.c |3 +--
> drivers/staging/rtl8192e/rtllib_softmac_wx.c |4 +---
> drivers/staging/rtl8192u/ieee80211/ieee80211_rx.c |3 +--
> drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c|3 +--
> drivers/staging/rtl8192u/ieee80211/ieee80211_softmac_wx.c |4 +---
> drivers/staging/rtl8192u/ieee80211/rtl819x_HTProc.c |3 +--
> drivers/staging/rtl8192u/r819xU_cmdpkt.c |3 +--
> drivers/staging/rtl8192u/r819xU_phy.c |3 +--
> drivers/staging/sep/sep_main.c|3 +--
> drivers/staging/silicom/bpctl_mod.c |4 ++--
> drivers/staging/wlan-ng/hfa384x_usb.c |3 +--
> drivers/staging/wlan-ng/p80211req.c |3 +--
> 18 files changed, 21 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c
> index ae7490b..777a13a 100644
> --- a/drivers/staging/bcm/Bcmchar.c
> +++ b/drivers/staging/bcm/Bcmchar.c
> @@ -1800,7 +1800,6 @@ static int bcm_char_ioctl_flash2x_section_bitmap(void
> __user *argp,
> {
> struct bcm_flash2x_bitmap *psFlash2xBitMap;
> struct bcm_ioctl_buffer IoBuffer;
> - INT Status = STATUS_FAILURE;
>
> BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL,
> "IOCTL_BCM_GET_FLASH2X_SECTION_BITMAP Called");
> @@ -1841,7 +1840,7 @@ BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG,
> DBG_LVL_ALL,
> }
>
> kfree(psFlash2xBitMap);
> - return Status;
> + return STATUS_FAILURE;
> }
>
> static int bcm_char_ioctl_set_active_section(void __user *argp,
> diff --git a/drivers/staging/bcm/InterfaceIdleMode.c
> b/drivers/staging/bcm/InterfaceIdleMode.c
> index fecf81f..c84ee49 100644
> --- a/drivers/staging/bcm/InterfaceIdleMode.c
> +++ b/drivers/staging/bcm/InterfaceIdleMode.c
> @@ -223,7 +223,6 @@ static int InterfaceAbortIdlemode(struct bcm_mini_adapter
> *Adapter,
> }
> int InterfaceIdleModeWakeup(struct bcm_mini_adapter *Adapter)
> {
> - ULONG Status = 0;
> if (Adapter->bTriedToWakeUpFromlowPowerMode) {
> BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS,
> IDLE_MODE, DBG_LVL_ALL,
> @@ -233,7 +232,7 @@ int InterfaceIdleModeWakeup(struct bcm_mini_adapter
> *Adapter)
> InterfaceAbortIdlemode(Adapter, Adapter->usIdleModePattern);
>
> }
> - return Status;
> + return 0;
> }
>
> void InterfaceHandleShutdownModeWakeup(struct bcm_mini_adapter *Adapter)
> diff --git a/drivers/staging/bcm/PHSModule.c b/drivers/staging/bcm/PHSModule.c
> index afc7bcc..07c5a0b 100644
> --- a/drivers/staging/bcm/PHSModule.c
> +++ b/drivers/staging/bcm/PHSModule.c
> @@ -409,7 +409,6 @@ ULONG PhsUpdateClassifierRule(IN void *pvContext,
> */
> ULONG PhsDeletePHSRule(IN void *pvContext, IN B_UINT16 uiVcid, IN B_UINT8
> u8PHSI)
> {
> - ULONG lStatus = 0;
> UINT nSFIndex = 0, nClsidIndex = 0;
> struct bcm_phs_entry *pstServiceFlowEntry = NULL;
> struct bcm_phs_classifier_table *pstClassifierRulesTable = NULL;
> @@ -446,7 +445,7 @@ ULONG PhsDeletePHSRule(IN void *pvContext, IN B_UINT16
> uiVcid, IN B_UINT8 u8PHSI
> }
> }
> }
> - return lStatus;
> + return 0;
> }
>
> /*
> @@ -467,7 +466,6 @@ ULONG PhsDeletePHSRule(IN void *pvContext, IN B_UINT16
> uiVcid, IN B_UINT8 u8PHSI
> */
> ULONG PhsDeleteClassifierRule(IN void *pvContext, IN B_UINT16 uiVcid, IN
> B_UINT16 uiClsId)
> {
> - ULONG lStatus = 0;
> UINT nSFIndex = 0, nClsidIndex = 0;
> struct bcm_phs_entry *pstServiceFlowEntry = NULL;
> struct bcm_phs_classifier_entry *pstClassifierEntry = NULL;
> @@ -504,7 +502,7 @@ ULONG PhsDeleteClassifierRule(IN void *pvContext, IN
> B_UINT16 uiVcid, IN B_UINT1
> memset(pstClassifierEntry, 0, sizeof(struct
> bcm_phs_classifier_entry));
> }
> }
> -
Re: [PATCH 8/8] staging: vt6655: Remove useless return variables
Am 20.05.2014 12:33, schrieb Peter Senna Tschudin:
> This patch remove variables that are initialized with a constant,
> are never updated, and are only used as parameter of return.
> Return the constant instead of using a variable.
>
> Verified by compilation only.
>
> The coccinelle script that find and fixes this issue is:
> //
> @@
> type T;
> constant C;
> identifier ret;
> @@
> - T ret = C;
> ... when != ret
> - return ret;
> + return C;
> //
>
> Signed-off-by: Peter Senna Tschudin
>
> ---
> drivers/staging/vt6655/hostap.c | 10 --
> drivers/staging/vt6655/wpactl.c | 19 ++-
> 2 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/staging/vt6655/hostap.c b/drivers/staging/vt6655/hostap.c
> index f55d0fb..317c2a8 100644
> --- a/drivers/staging/vt6655/hostap.c
> +++ b/drivers/staging/vt6655/hostap.c
> @@ -421,7 +421,6 @@ static int hostap_set_encryption(PSDevice pDevice,
> unsigned char abySeq[MAX_KEY_LEN];
> unsigned long long KeyRSC;
> unsigned char byKeyDecMode = KEY_CTL_WEP;
> - int ret = 0;
> int iNodeIndex = -1;
> int ii;
> bool bKeyTableFull = false;
> @@ -475,7 +474,7 @@ static int hostap_set_encryption(PSDevice pDevice,
> MAX_KEY_LEN
> );
>
> - return ret;
> + return 0;
> }
>
> memcpy(abyKey, param->u.crypt.key, param->u.crypt.key_len);
> @@ -531,7 +530,7 @@ static int hostap_set_encryption(PSDevice pDevice,
> pMgmt->byCSSGK = KEY_CTL_WEP;
> pMgmt->sNodeDBTable[iNodeIndex].byCipherSuite = KEY_CTL_WEP;
> pMgmt->sNodeDBTable[iNodeIndex].dwKeyIndex = dwKeyIndex;
> - return ret;
> + return 0;
> }
>
> if (param->u.crypt.seq) {
> @@ -622,7 +621,7 @@ static int hostap_set_encryption(PSDevice pDevice,
> pMgmt->sNodeDBTable[iNodeIndex].dwTSC47_16 = 0;
> pMgmt->sNodeDBTable[iNodeIndex].wTSC15_0 = 0;
>
> - return ret;
> + return 0;
> }
>
> /*
> @@ -643,7 +642,6 @@ static int hostap_get_encryption(PSDevice pDevice,
>int param_len)
> {
> PSMgmtObjectpMgmt = pDevice->pMgmt;
> - int ret = 0;
> int ii;
> int iNodeIndex = 0;
>
> @@ -663,7 +661,7 @@ static int hostap_get_encryption(PSDevice pDevice,
> for (ii = 0; ii < 8; ii++)
> param->u.crypt.seq[ii] = (unsigned
> char)pMgmt->sNodeDBTable[iNodeIndex].KeyRSC >> (ii * 8);
>
> - return ret;
> + return 0;
> }
>
> /*
> diff --git a/drivers/staging/vt6655/wpactl.c b/drivers/staging/vt6655/wpactl.c
> index 089e8a7..8392d4d 100644
> --- a/drivers/staging/vt6655/wpactl.c
> +++ b/drivers/staging/vt6655/wpactl.c
> @@ -411,12 +411,11 @@ static int wpa_set_wpa(PSDevice pDevice,
> struct viawget_wpa_param *param)
> {
> PSMgmtObjectpMgmt = pDevice->pMgmt;
> - int ret = 0;
>
> pMgmt->eAuthenMode = WMAC_AUTH_OPEN;
> pMgmt->bShareKeyAlgorithm = false;
>
> - return ret;
> + return 0;
> }
perhaps this can be void ?
>
> /*
> @@ -437,7 +436,6 @@ static int wpa_set_disassociate(PSDevice pDevice,
> struct viawget_wpa_param *param)
> {
> PSMgmtObjectpMgmt = pDevice->pMgmt;
> - int ret = 0;
>
> spin_lock_irq(&pDevice->lock);
> if (pDevice->bLinkPass) {
> @@ -446,7 +444,7 @@ static int wpa_set_disassociate(PSDevice pDevice,
> }
> spin_unlock_irq(&pDevice->lock);
>
> - return ret;
> + return 0;
> }
>
> /*
> @@ -466,14 +464,12 @@ static int wpa_set_disassociate(PSDevice pDevice,
> static int wpa_set_scan(PSDevice pDevice,
> struct viawget_wpa_param *param)
> {
> - int ret = 0;
> -
> spin_lock_irq(&pDevice->lock);
> BSSvClearBSSList((void *)pDevice, pDevice->bLinkPass);
> bScheduleCommand((void *)pDevice, WLAN_CMD_BSSID_SCAN, NULL);
> spin_unlock_irq(&pDevice->lock);
>
> - return ret;
> + return 0;
> }
This could be void ?
> /*
> @@ -494,11 +490,10 @@ static int wpa_get_bssid(PSDevice pDevice,
>struct viawget_wpa_param *param)
> {
> PSMgmtObjectpMgmt = pDevice->pMgmt;
> - int ret = 0;
>
> memcpy(param->u.wpa_associate.bssid, pMgmt->abyCurrBSSID , 6);
>
> - return ret;
> + return 0;
> }
this could be void
>
> /*
> @@ -520,14 +515,13 @@ static int wpa_get_ssid(PSDevice pDevice,
> {
> PSMgmtObjectpMgmt = pDevice->pMgmt;
> PWLAN_IE_SSID pItemSSID;
> - int ret = 0;
>
> pItemSSID = (PWLAN_IE_SSID)pMgmt->abyCurrSSID;
>
> memcpy(param->u.wpa_associate.ssid, pItemSSID->abySSID ,
> pItemSSID->len);
> param->u.wpa_associate.ssid_len = pItemSSID->len;
>
> - return ret;
> + return 0;
> }
also void ?
> /*
> @@ -668,7 +662,6 @@ static int wpa_set_associate(PSDevice pDevice
Re: [PATCH 1/8] staging: r8712u: Remove useless return variables
Am 20.05.2014 13:41, schrieb Dan Carpenter: > Those concerns are valid but the code was like that in the original so > we should merge this patch as is and hope some volunteer will fix things > up in a follow on patch. > > Fixing them in this patch would be a mistake anyway because of the one > thing per patch rule. > I see this as a bordercase, the patch from Peter is correct in the context of removing useless return variables. I question the whole function in the hope that the maintainer will decide that the function can go completely. re, wh ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: lustre: Fix non-ANSI sparse warnings
Am 28.07.2013 00:38, schrieb Emil Goode:
> This patch fixes a lot of non-ANSI sparse warnings by adding void to
> parameterless functions.
>
> Signed-off-by: Emil Goode
> ---
> v2: A bit more specific commit message.
>
> .../lustre/lnet/klnds/socklnd/socklnd_lib-linux.c|6 +++---
> drivers/staging/lustre/lnet/lnet/api-ni.c|2 +-
> drivers/staging/lustre/lnet/selftest/console.c |2 +-
> .../staging/lustre/lustre/libcfs/linux/linux-tracefile.c | 14
> +++---
> drivers/staging/lustre/lustre/ptlrpc/pack_generic.c |2 +-
> drivers/staging/lustre/lustre/ptlrpc/pinger.c|4 ++--
> 6 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.c
> b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.c
> index 3e08fe2..76e442b 100644
> --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.c
> +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.c
> @@ -325,20 +325,20 @@ ksocknal_lib_tunables_init ()
> }
>
> void
> -ksocknal_lib_tunables_fini ()
> +ksocknal_lib_tunables_fini(void)
> {
> if (ksocknal_tunables.ksnd_sysctl != NULL)
> unregister_sysctl_table(ksocknal_tunables.ksnd_sysctl);
> }
this is not needed as unregister_sysctl_table() can handle NULL
> #else
> int
> -ksocknal_lib_tunables_init ()
> +ksocknal_lib_tunables_init(void)
> {
> return 0;
> }
>
> void
> -ksocknal_lib_tunables_fini ()
> +ksocknal_lib_tunables_fini(void)
> {
> }
> #endif /* # if CONFIG_SYSCTL && !CFS_SYSFS_MODULE_PARM */
> diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c
> b/drivers/staging/lustre/lnet/lnet/api-ni.c
> index 250c618..160a429 100644
> --- a/drivers/staging/lustre/lnet/lnet/api-ni.c
> +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c
> @@ -1371,7 +1371,7 @@ EXPORT_SYMBOL(LNetNIInit);
> * \return always 0 for current implementation.
> */
> int
> -LNetNIFini()
> +LNetNIFini(void)
> {
> LNET_MUTEX_LOCK(&the_lnet.ln_api_mutex);
>
> diff --git a/drivers/staging/lustre/lnet/selftest/console.c
> b/drivers/staging/lustre/lnet/selftest/console.c
> index 78e8d04..09e4700 100644
> --- a/drivers/staging/lustre/lnet/selftest/console.c
> +++ b/drivers/staging/lustre/lnet/selftest/console.c
> @@ -1773,7 +1773,7 @@ lstcon_session_info(lst_sid_t *sid_up, int *key_up,
> unsigned *featp,
> }
>
> int
> -lstcon_session_end()
> +lstcon_session_end(void)
> {
> lstcon_rpc_trans_t *trans;
> lstcon_group_t *grp;
> diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c
> b/drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c
> index a500a0b..162beee 100644
> --- a/drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c
> +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-tracefile.c
> @@ -51,7 +51,7 @@ char *cfs_trace_console_buffers[NR_CPUS][CFS_TCD_TYPE_MAX];
>
> struct rw_semaphore cfs_tracefile_sem;
>
> -int cfs_tracefile_init_arch()
> +int cfs_tracefile_init_arch(void)
> {
> inti;
> intj;
> @@ -96,7 +96,7 @@ out:
> return -ENOMEM;
> }
>
> -void cfs_tracefile_fini_arch()
> +void cfs_tracefile_fini_arch(void)
> {
> inti;
> intj;
> @@ -116,27 +116,27 @@ void cfs_tracefile_fini_arch()
> fini_rwsem(&cfs_tracefile_sem);
> }
>
> -void cfs_tracefile_read_lock()
> +void cfs_tracefile_read_lock(void)
> {
> down_read(&cfs_tracefile_sem);
> }
>
> -void cfs_tracefile_read_unlock()
> +void cfs_tracefile_read_unlock(void)
> {
> up_read(&cfs_tracefile_sem);
> }
>
> -void cfs_tracefile_write_lock()
> +void cfs_tracefile_write_lock(void)
> {
> down_write(&cfs_tracefile_sem);
> }
>
> -void cfs_tracefile_write_unlock()
> +void cfs_tracefile_write_unlock(void)
> {
> up_write(&cfs_tracefile_sem);
> }
>
> -cfs_trace_buf_type_t cfs_trace_buf_idx_get()
> +cfs_trace_buf_type_t cfs_trace_buf_idx_get(void)
> {
> if (in_irq())
> return CFS_TCD_TYPE_IRQ;
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
> b/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
> index 648..9c20d0c 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/pack_generic.c
> @@ -115,7 +115,7 @@ int lustre_msg_check_version(struct lustre_msg *msg,
> __u32 version)
> EXPORT_SYMBOL(lustre_msg_check_version);
>
> /* early reply size */
> -int lustre_msg_early_size()
> +int lustre_msg_early_size(void)
> {
> static int size = 0;
> if (!size) {
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/pinger.c
> b/drivers/staging/lustre/lustre/ptlrpc/pinger.c
> index f521251..79e7d08 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/pinger.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/pinger.c
> @@ -51,7 +51,7 @@ struct mutex pinger_mutex;
> static LIST_HEAD(pinger_imports);
> static st
S u p p l y
G r e e t i n g s My name is Maj. Walter Johnson, an American Army, currently in Syria for peacekeeping duty. I wish to let you know about a business of supplying Pharmaceutical active ingredients from India to an American company, I have done this business with an Indian partner until last when I lost him. There is an active raw ingredients oil, which I supplied to the American company from India through an Indian partner. Right now I am in Syria for a peacekeeping mission, secondly the Coronavirus problem, so I cannot execute the business by myself hence I'm seeking your partnership, the Director of the American company has asked me for the contact of the India dealer to enable them to contact them for the supply. Because of the Corona-Virus pandemic, they told me that they cannot send their Manager to India, but they will send money to their agent in the American Embassy in India so that he can get the product from the seller and make payment. Afterwards, the agent will courier the product to America through their freight agent. Last year the American Company sent their purchase manager to India, but due to the recent global pandemic Corona Virus Threat, the American Company opted to send money to the agent to get the product and cargo to them to American. I do not wish to give the American company's Managing Director or Chief Purchase Officer, the contact of the Indian Seller, because of my profit in the business, I normally made good profits and it was too encouraging to give away, I intended to present you as the Indian vendor to the American company (That means that you will stand as a middle person between the American company and original Indian vendor) so that the American company will not know the source and real cost of the stuff. The Original Indian vendor is selling the material at the cost of 4,370USD Per Litre, while the American company buys at the cost of 13,890USD Per Litre. The Company normally demands between 1000- 1500 Litres every three months. I will convince the American company to buy the material from you. We will get the material from the Indian vendor and supply it to the American company through their agent over there in India. Your duty will be to get the material from the local dealer and supply to the American Company then we (you and I ) will share the profit made on the basis of 70% for you and 30% for me. Your role must be played perfectly and the least I will expect from you is betrayal. Again, I don║╞t want the American company to know the real cost or source of the product, likewise I don't want the Indian vendor to know about the American company. A w a i t i n g your response Maj. Walter Johnson ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
S u p p l y
G r e e t i n g s My name is Maj. Walter Johnson, an American Army, currently in Syria for peacekeeping duty. I wish to let you know about a business of supplying Pharmaceutical active ingredients from India to an American company, I have done this business with an Indian partner until last when I lost him. There is an active raw ingredients oil, which I supplied to the American company from India through an Indian partner. Right now I am in Syria for a peacekeeping mission, secondly the Coronavirus problem, so I cannot execute the business by myself hence I'm seeking your partnership, the Director of the American company has asked me for the contact of the India dealer to enable them to contact them for the supply. Because of the Corona-Virus pandemic, they told me that they cannot send their Manager to India, but they will send money to their agent in the American Embassy in India so that he can get the product from the seller and make payment. Afterwards, the agent will courier the product to America through their freight agent. Last year the American Company sent their purchase manager to India, but due to the recent global pandemic Corona Virus Threat, the American Company opted to send money to the agent to get the product and cargo to them to American. I do not wish to give the American company's Managing Director or Chief Purchase Officer, the contact of the Indian Seller, because of my profit in the business, I normally made good profits and it was too encouraging to give away, I intended to present you as the Indian vendor to the American company (That means that you will stand as a middle person between the American company and original Indian vendor) so that the American company will not know the source and real cost of the stuff. The Original Indian vendor is selling the material at the cost of 4,370USD Per Litre, while the American company buys at the cost of 13,890USD Per Litre. The Company normally demands between 1000- 1500 Litres every three months. I will convince the American company to buy the material from you. We will get the material from the Indian vendor and supply it to the American company through their agent over there in India. Your duty will be to get the material from the local dealer and supply to the American Company then we (you and I ) will share the profit made on the basis of 70% for you and 30% for me. Your role must be played perfectly and the least I will expect from you is betrayal. Again, I don’t want the American company to know the real cost or source of the product, likewise I don't want the Indian vendor to know about the American company. A w a i t i n g your response Maj. Walter Johnson ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
