issue with uninitialized value used in a comparison in gbcodec_mixer_dapm_ctl_put

[Colin Ian King]

Hi,

Static analysis with Coverity has detected an uninitialized value being
used in a comparison.  The error was detected on a recent change to
drivers/staging/greybus/audio_topology.c however the issue actually
dates back to the original commit:

commit 6339d2322c47f4b8ebabf9daf0130328ed72648b
Author: Vaibhav Agarwal 
Date:   Wed Jan 13 14:07:51 2016 -0700

greybus: audio: Add topology parser for GB codec

The analysis is as follows:

425 static int gbcodec_mixer_dapm_ctl_put(struct snd_kcontrol *kcontrol,
426  struct snd_ctl_elem_value
*ucontrol)
427 {
428int ret, wi, max, connect;
429unsigned int mask, val;
430struct gb_audio_ctl_elem_info *info;
431struct gbaudio_ctl_pvt *data;

   1. var_decl: Declaring variable gbvalue without initializer.
432struct gb_audio_ctl_elem_value gbvalue;
433struct gbaudio_module_info *module;
434struct snd_soc_dapm_widget_list *wlist =
snd_kcontrol_chip(kcontrol);
435struct snd_soc_dapm_widget *widget = wlist->widgets[0];
436struct device *codec_dev = widget->dapm->dev;
437struct gbaudio_codec_info *gb = dev_get_drvdata(codec_dev);
438struct gb_bundle *bundle;
439

   2. Condition 0 /* __builtin_types_compatible_p() */, taking false branch.
   3. Condition 1 /* __builtin_types_compatible_p() */, taking true branch.
   4. Falling through to end of if statement.
   5. Condition !!branch, taking false branch.
   6. Condition ({...; !!branch;}), taking false branch.

440dev_dbg(codec_dev, "Entered %s:%s\n", __func__,
kcontrol->id.name);
441module = find_gb_module(gb, kcontrol->id.name);

   7. Condition !module, taking false branch.
442if (!module)
443return -EINVAL;
444
445data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
446info = (struct gb_audio_ctl_elem_info *)data->info;

   8. Condition 0 /* !!(!__builtin_types_compatible_p() &&
!__builtin_types_compatible_p()) */, taking false branch.
447bundle = to_gb_bundle(module->dev);
448

   9. Condition data->vcount == 2, taking true branch.
449if (data->vcount == 2)
450dev_warn(widget->dapm->dev,
451 "GB: Control '%s' is stereo, which is not
supported\n",
452 kcontrol->id.name);
453
454max = le32_to_cpu(info->value.integer.max);
455mask = (1 << fls(max)) - 1;
456val = ucontrol->value.integer.value[0] & mask;

   10. Condition !!val, taking true branch.
457connect = !!val;
458
459/* update ucontrol */

Uninitialized scalar variable (UNINIT)
   11. uninit_use: Using uninitialized value gbvalue.value.integer_value[0].
460if (gbvalue.value.integer_value[0] != val) {

The gbvalue.value.integer_value[0] read is bogus since gbvalue was
declared on the stack but was not initialized.  There seems to be no
where that sets this data. I'm assuming most of the time that the
comparison works because the garbage value is different from val and so
the code in the if stanza is executed.

Anyhow, I'm unsure what the original intent of the code was, so I've not
attempted to fix this.

Colin


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: exfat: issue with FFS_MEDIAERR error return assignment

[Colin Ian King]

On 10/09/2019 13:44, Dan Carpenter wrote:
> On Fri, Aug 30, 2019 at 07:38:00PM +0100, Colin Ian King wrote:
>> Hi,
>>
>> Static analysis on exfat with Coverity has picked up an assignment of
>> FFS_MEDIAERR that gets over-written:
>>
>>
>> 1750if (is_dir) {
>> 1751if ((fid->dir.dir == p_fs->root_dir) &&
>> 1752(fid->entry == -1)) {
>> 1753if (p_fs->dev_ejected)
> 
> Idealy we would have both a filename and a function name but this email
> doesn't have either so no one knows what code you are talking about.  :P

Oops, my bad.

drivers/staging/exfat/exfat_super.c ffsWriteStat()

> 
> regards,
> dan carpenter
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8723bs: hal: remove redundant assignment to variable n

[Colin Ian King]

On 05/09/2019 15:52, Dan Carpenter wrote:
> It would be better to remove "n" altogether.

Good point, will resend a V2 later.
> 
> regards,
> dan carpenter
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

staging: exfat: issue with FFS_MEDIAERR error return assignment

[Colin Ian King]

Hi,

Static analysis on exfat with Coverity has picked up an assignment of
FFS_MEDIAERR that gets over-written:


1750if (is_dir) {
1751if ((fid->dir.dir == p_fs->root_dir) &&
1752(fid->entry == -1)) {
1753if (p_fs->dev_ejected)

CID 85797 (#1 of 1): Unused value (UNUSED_VALUE)
Assigning value 1 to ret here, but that stored value is overwritten
before it can be used.

1754ret = FFS_MEDIAERR;

value_overwrite: Overwriting previous write to ret with value 0.

1755ret = FFS_SUCCESS;
1756goto out;
1757}
1758}

I doubt that's intentional, should it be instead the following?

if (p_fs->dev_ejected)
ret = FFS_MEDIAERR;
else
ret = FFS_SUCCESS;
goto out;

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

re: Added Realtek rtl8192u driver to staging - static analysis report.

[Colin Ian King]

Hi,

Static analysis of linux-next picked up an issue with the following commit:

commit 8fc8598e61f6f384f3eaf1d9b09500c12af47b37
Author: Jerry Chuang 
Date:   Tue Nov 3 07:17:11 2009 -0200

Staging: Added Realtek rtl8192u driver to staging

In drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c we have:

CID 48331 (#1 of 1): Unused value (UNUSED_VALUE) assigned_pointer

Assigning value from ieee->crypt[ieee->tx_keyidx] to crypt here, but
that stored value is not used.

746crypt = ieee->crypt[ieee->tx_keyidx];
747if (encrypt)
748beacon_buf->capability |=
cpu_to_le16(WLAN_CAPABILITY_PRIVACY);

Pointer crypt is being assigned but is never used afterwards.  Now
either this is a redundant assignment and can be removed OR maybe crypt
should be checked and there is a typo, e.g.:

crypt = ieee->crypt[ieee->tx_keyidx];
if (crypt)
...

Either way, it's not clear to me and I think the code needs cleaning up.
Any ideas?

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

static analysis bug report in drivers/staging/iio/dac/ad5380.c

[Colin Ian King]

Hi,

Static analysis with Coverity Scan has detected a potential assignment
bug in ad5380.c:

217case IIO_CHAN_INFO_CALIBBIAS:
218ret = regmap_read(st->regmap,
AD5380_REG_OFFSET(chan->address),
219val);
220if (ret)
221return ret;
222*val >>= chan->scan_type.shift;

CID 43178 (#1 of 1): Unused value (UNUSED_VALUE)assigned_pointer:
Assigning value from val - (1 << chan->scan_type.realbits) / 2 to val
here, but that stored value is not used.

223val -= (1 << chan->scan_type.realbits) / 2;
224return IIO_VAL_INT;

val is a pointer and so updating it before a return is probably not the
intention.  I suspect the intention was probably:

   *val -= (1 << chan->scan_type.realbits) / 2;

However, I'm not confident about this as the following case has:

225case IIO_CHAN_INFO_SCALE:
226*val = 2 * st->vref;
227*val2 = chan->scan_type.realbits;
228return IIO_VAL_FRACTIONAL_LOG2;

which may imply the update maybe to *val2 instead, e.g.:

*val2 -= (1 << chan->scan_type.realbits) / 2;

Any ideas?

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

re: media: staging/imx: Improve pipeline searching (bug report)

[Colin Ian King]

Hi,

Static analysis with Coverity on Linux next has found a potential issue
with the following commit:

commit 3ef46bc97ca2c918b7657a08220c7340a9bb07a2
Author: Steve Longerbeam 
Date:   Fri May 10 17:50:11 2019 -0400

media: staging/imx: Improve pipeline searching


The issue is in drivers/staging/media/imx/imx-media-utils.c in function
find_pipeline_entity:

struct media_pad *pad = NULL;

pad is assigned a NULL

struct video_device *vfd;
struct v4l2_subdev *sd;

if (grp_id && is_media_entity_v4l2_subdev(start)) {
sd = media_entity_to_v4l2_subdev(start);
if (sd->grp_id & grp_id)
return >entity;
} else if (buftype && is_media_entity_v4l2_video_device(start)) {
vfd = media_entity_to_video_device(pad->entity);

..and above the null pad is being dereferenced causing a kernel oops.

if (buftype == vfd->queue->type)
return >entity;
}


Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8723bs: remove redundant assignment to rtStatus

[Colin Ian King]

On 17/06/2019 13:47, Colin King wrote:
> From: Colin Ian King 
> 
> Variable rtStatus is initialized with a value that is never read
> and later it is reassigned a new value.  Hence the initialization
> is redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c 
> b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> index 21f2365fa627..bda19769c37f 100644
> --- a/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> +++ b/drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c
> @@ -352,7 +352,7 @@ void rtl8723b_FirmwareSelfReset(struct adapter *padapter)
>  /*  */
>  s32 rtl8723b_FirmwareDownload(struct adapter *padapter, bool  bUsedWoWLANFw)
>  {
> - s32 rtStatus = _SUCCESS;
> + s32 rtStatus;
>   u8 write_fw = 0;
>   unsigned long fwdl_start_time;
>   struct hal_com_data *pHalData = GET_HAL_DATA(padapter);
> 

Actually, ignore this, the driver has lots more of these that need
cleaning up, I'll send a V2 with all the fixups later.

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

staging: Add rtl8723bs sdio wifi driver

[Colin Ian King]

Hi,

static analysis with Coverity has detected an issues in the rtl8723bs
wifi driver:

File: drivers/staging/rtl8723bs/os_dep/ioctl_linux.c in function
rtw_dbg_port():

CID 18480: Operands don't affect result (CONSTANT_EXPRESSION_RESULT)
dead_error_condition: The condition (extra_arg & 7U) > 7U cannot be true.

if ((extra_arg & 0x07) > 0x07)
padapter->driver_ampdu_spacing = 0xFF;
else
padapter->driver_ampdu_spacing = extra_arg;


I'm not sure if the mask is (in)correct or the value it is being
compared to 0x07 is (in)correct (or both!)

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ks7010: remove redundant auth_type check

[Colin Ian King]

On 19/03/2019 13:29, Greg Kroah-Hartman wrote:
> On Mon, Mar 18, 2019 at 09:32:59PM +, Jeremy Sowden wrote:
>> On 2019-03-18, at 10:57:49 +, Colin King wrote:
>>> The range check on auth_type is redundant as there is a prior
>>> check on the auth_type values and the only way the block is entered
>>> is if auth_type is one of TYPE_PMK1, TYPE_GMK1 and TYPE_GMK1.
>>
>> "... and TYPE_GMK2."
> 
> I'll go edit that, thanks.
> 
> greg k-h
> 
Thanks for fixing my typo.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

static analysis bug report: staging: rtl8192u: use of uninitialized array

[Colin Ian King]

Hi,

static analysis with cppcheck has detected use of an uninitialized array
tmp_ssid in drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c in
function ieee80211_softmac_new_net()

Array tmp_ssid is only initialized when ssidbroad is non-null, however
it is being copied and the copy of this is being printk'd later on:

if (!ssidbroad) {
strncpy(tmp_ssid,
ieee->current_network.ssid, IW_ESSID_MAX_SIZE);

tmp_ssid is initialized only here ^^

tmp_ssid_len =
ieee->current_network.ssid_len;
}
memcpy(>current_network, net,
sizeof(struct ieee80211_network));

strncpy(ieee->current_network.ssid, tmp_ssid,
IW_ESSID_MAX_SIZE);

tmp_ssid is being copied from here ^^

ieee->current_network.ssid_len = tmp_ssid_len;
printk(KERN_INFO"Linking with %s,channel:%d,
qos:%d, myHT:%d, networkHT:%d\n",
   ieee->current_network.ssid,
   ieee->current_network.channel,
   ieee->current_network.qos_data.supported,
   ieee->pHTInfo->bEnableHT,
   ieee->current_network.bssht.bdSupportHT);

copy of tmp_ssid is being printk'd here ^^

So potentially a garbage non-null terminated string is being printk'd.
Not sure what the fix is for the case where ssidbroad is non-null, what
should tmp_ssid be in that situation?

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

static analysis bug report: staging r8712u memcpy of uninitialized variable

[Colin Ian King]

Hi,

Static analysis with cppcheck found a couple of interesting issues with
memcpy'ing of an uninitialized variable. Two occurrences of the same
issue are found in drivers/staging/rtl8712/rtl8712_cmd.c in functions
read_bbreg_hdl and read_rfreg_hdl.

For example:

static u8 read_bbreg_hdl(struct _adapter *padapter, u8 *pbuf)
{
u32 val;
void (*pcmd_callback)(struct _adapter *dev, struct cmd_obj
*pcmd);
struct cmd_obj *pcmd  = (struct cmd_obj *)pbuf;

if (pcmd->rsp && pcmd->rspsz > 0)
memcpy(pcmd->rsp, (u8 *), pcmd->rspsz);



}

I don't understand why the contents of val is being memcpy'd to
pcmd->rsp, especially when val is uninitialized and hence contains
garbage. Any ideas?

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: davinci: drop pointless static qualifier in vpfe_resizer_init()

[Colin Ian King]

On 11/03/2019 14:07, Dan Carpenter wrote:
> On Mon, Mar 11, 2019 at 10:14:05PM +0800, Mao Wenan wrote:
>> There is no need to have the 'T *v' variable static
>> since new value always be assigned before use it.
>>
>> Signed-off-by: Mao Wenan 
>> ---
>>  drivers/staging/media/davinci_vpfe/dm365_resizer.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c 
>> b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
>> index 6098f43ac51b..a2a672d4615d 100644
>> --- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
>> +++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
>> @@ -1881,7 +1881,7 @@ int vpfe_resizer_init(struct vpfe_resizer_device 
>> *vpfe_rsz,
>>  struct v4l2_subdev *sd = _rsz->crop_resizer.subdev;
>>  struct media_pad *pads = _rsz->crop_resizer.pads[0];
>>  struct media_entity *me = >entity;
>> -static resource_size_t  res_len;
>> +resource_size_t  res_len;
> ^
> Could you remove the extra space character also, please.

shouldn't checkpatch find these?

> 
>>  struct resource *res;
>>  int ret;
> 
> regards,
> dan carpenter
> 
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8192u: fix a null pointer dereference on a null dev pointer

[Colin Ian King]

On 03/02/2019 11:31, Dan Carpenter wrote:
> On Sat, Feb 02, 2019 at 10:56:27PM +, Colin King wrote:
>> From: Colin Ian King 
>>
>> There is an earlier null check on pointer dev which implies it may be null,
>> however the assignment of pointer pref and the call to free_ieee82011 on
>> a null dev can cause null pointer dereference errors.  Fix this by moving
>> the assignment of priv and the the call to free_ieee80211 into the block of
>> code that performs the null dev sanity check.
>>
>> Detected by CoverityScan, CID#143078 ("Dereference after null check")
>>
>> Fixes: 8fc8598e61f6 ("Staging: Added Realtek rtl8192u driver to staging")
>> Signed-off-by: Colin Ian King 
>> ---
>>  drivers/staging/rtl8192u/r8192U_core.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8192u/r8192U_core.c 
>> b/drivers/staging/rtl8192u/r8192U_core.c
>> index 0ac0bbf7d923..4741a29326ea 100644
>> --- a/drivers/staging/rtl8192u/r8192U_core.c
>> +++ b/drivers/staging/rtl8192u/r8192U_core.c
>> @@ -4955,9 +4955,10 @@ static void rtl8192_cancel_deferred_work(struct 
>> r8192_priv *priv)
>>  static void rtl8192_usb_disconnect(struct usb_interface *intf)
>>  {
>>  struct net_device *dev = usb_get_intfdata(intf);
>> -struct r8192_priv *priv = ieee80211_priv(dev);
>>  
>>  if (dev) {
>> +struct r8192_priv *priv = ieee80211_priv(dev);
> 
> "dev" can't actually be NULL.  Look how we call usb_set_intfdata() in
> probe().  It's better to remove the check instead.

Yep, good point. I'll send a V2.

> 
> regards,
> dan carpenter
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH][next] media: staging: intel-ipu3: fix unsigned comparison with < 0

[Colin Ian King]

ping?

On 22/12/2018 11:49, Colin King wrote:
> From: Colin Ian King 
> 
> The comparison css->pipes[pipe].bindex < 0 is always false because
> bindex is an unsigned int.  Fix this by using a signed integer for
> the comparison.
> 
> Detected by CoverityScan, CID#1476023 ("Unsigned compared against 0")
> 
> Fixes: f5f2e4273518 ("media: staging/intel-ipu3: Add css pipeline 
> programming")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/staging/media/ipu3/ipu3-css.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu3/ipu3-css.c 
> b/drivers/staging/media/ipu3/ipu3-css.c
> index 44c55639389a..b9354d2bb692 100644
> --- a/drivers/staging/media/ipu3/ipu3-css.c
> +++ b/drivers/staging/media/ipu3/ipu3-css.c
> @@ -1751,7 +1751,7 @@ int ipu3_css_fmt_try(struct ipu3_css *css,
>   [IPU3_CSS_QUEUE_OUT].fmt.mpix;
>   struct v4l2_pix_format_mplane *const vf =
>   [IPU3_CSS_QUEUE_VF].fmt.mpix;
> - int i, s;
> + int i, s, ret;
>  
>   /* Adjust all formats, get statistics buffer sizes and formats */
>   for (i = 0; i < IPU3_CSS_QUEUES; i++) {
> @@ -1826,12 +1826,12 @@ int ipu3_css_fmt_try(struct ipu3_css *css,
>   s = (bds->height - gdc->height) / 2 - FILTER_SIZE;
>   env->height = s < MIN_ENVELOPE ? MIN_ENVELOPE : s;
>  
> - css->pipes[pipe].bindex =
> - ipu3_css_find_binary(css, pipe, q, r);
> - if (css->pipes[pipe].bindex < 0) {
> + ret = ipu3_css_find_binary(css, pipe, q, r);
> + if (ret < 0) {
>   dev_err(css->dev, "failed to find suitable binary\n");
>   return -EINVAL;
>   }
> + css->pipes[pipe].bindex = ret;
>  
>   dev_dbg(css->dev, "Binary index %d for pipe %d found.",
>   css->pipes[pipe].bindex, pipe);
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [greybus-dev] [PATCH] staging: greybus: fix spelling mistake "entires" -> "entries"

[Colin Ian King]

On 14/09/18 12:43, Alex Elder wrote:
> On 09/14/2018 06:24 AM, Colin King wrote:
>> From: Colin Ian King 
>>
>> Trivial fix to spelling mistake
> 
> I hate to have two-character fixes to documentation like this.  I.e.,
> as long as you're touching the README file it might have been nice to
> review it and look for other things.  I suspect you found this in the
> program output though, and also noticed it in the README file.

Indeed, it was the program output where the mistake was first spotted.

> 
> So...  Looks good to me.
> 
> Reviewed-by: Alex Elder 
> 
>>
>> Signed-off-by: Colin Ian King 
>> ---
>>  drivers/staging/greybus/tools/README.loopback | 2 +-
>>  drivers/staging/greybus/tools/loopback_test.c | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/greybus/tools/README.loopback 
>> b/drivers/staging/greybus/tools/README.loopback
>> index 845b08dc4696..070a510cbe7c 100644
>> --- a/drivers/staging/greybus/tools/README.loopback
>> +++ b/drivers/staging/greybus/tools/README.loopback
>> @@ -79,7 +79,7 @@ Here is the summary of the available options:
>> -t must be one of the test names - sink, transfer or ping
>> -i iteration count - the number of iterations to run the test over
>>   Optional arguments
>> -   -S sysfs location - location for greybus 'endo' entires default 
>> /sys/bus/greybus/devices/
>> +   -S sysfs location - location for greybus 'endo' entries default 
>> /sys/bus/greybus/devices/
>> -D debugfs location - location for loopback debugfs entries default 
>> /sys/kernel/debug/gb_loopback/
>> -s size of data packet to send during test - defaults to zero
>> -m mask - a bit mask of connections to include example: -m 8 = 4th 
>> connection -m 9 = 1st and 4th connection etc
>> diff --git a/drivers/staging/greybus/tools/loopback_test.c 
>> b/drivers/staging/greybus/tools/loopback_test.c
>> index b82e2befe935..2fa88092514d 100644
>> --- a/drivers/staging/greybus/tools/loopback_test.c
>> +++ b/drivers/staging/greybus/tools/loopback_test.c
>> @@ -192,7 +192,7 @@ void usage(void)
>>  "   -t must be one of the test names - sink, transfer or ping\n"
>>  "   -i iteration count - the number of iterations to run the test 
>> over\n"
>>  " Optional arguments\n"
>> -"   -S sysfs location - location for greybus 'endo' entires default 
>> /sys/bus/greybus/devices/\n"
>> +"   -S sysfs location - location for greybus 'endo' entries default 
>> /sys/bus/greybus/devices/\n"
>>  "   -D debugfs location - location for loopback debugfs entries 
>> default /sys/kernel/debug/gb_loopback/\n"
>>  "   -s size of data packet to send during test - defaults to zero\n"
>>  "   -m mask - a bit mask of connections to include example: -m 8 = 
>> 4th connection -m 9 = 1st and 4th connection etc\n"
>>
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: most: video: fix registration of an empty comp core_component

[Colin Ian King]

On 05/09/18 11:06, Alexander Stein wrote:
> On Wednesday, September 5, 2018, 11:46:05 AM CEST Colin King wrote:
>> From: Colin Ian King 
>>
>> Currently we have structrues comp (which is empty) and comp_info being
>> used to register and deregister the component.  This mismatch in naming
>> occurred from a previous commit that renamed aim_info to comp. Fix this
>> to use consistent component naming in line with most/net, most/sound etc.
>>
>> This fixes the message two issues, one with a null empty name when
>> loading the module:
>>
>> [ 1485.269515] most_core: registered new core component (null)
>>
>> and an Oops when removing the module:
>>
>> [ 1485.277971] BUG: unable to handle kernel NULL pointer dereference at 
>> 0008
>> [ 1485.278648] PGD 0 P4D 0
>> [ 1485.279253] Oops: 0002 [#2] SMP PTI
>> [ 1485.279847] CPU: 1 PID: 32629 Comm: modprobe Tainted: P  D WC OE 
>> 4.18.0-8-generic #9
>> [ 1485.280442] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
>> 0.0.0 02/06/2015
>> [ 1485.281040] RIP: 0010:most_deregister_component+0x3c/0x70 [most_core]
>> .. etc
>>
>> Fixes: 1b10a0316e2d ("staging: most: video: remove aim designators")
>> Signed-off-by: Colin Ian King 
>> ---
>>  drivers/staging/most/video/video.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/most/video/video.c 
>> b/drivers/staging/most/video/video.c
>> index cf342eb58e10..ad7e28ab9a4f 100644
>> --- a/drivers/staging/most/video/video.c
>> +++ b/drivers/staging/most/video/video.c
>> @@ -530,7 +530,7 @@ static int comp_disconnect_channel(struct most_interface 
>> *iface,
>>  return 0;
>>  }
>>  
>> -static struct core_component comp_info = {
>> +static struct core_component comp = {
>>  .name = "video",
>>  .probe_channel = comp_probe_channel,
>>  .disconnect_channel = comp_disconnect_channel,
> 
> Doesn't it make more sense to move that variable defintion where currently 
> the forward declaration is?

Probably, I was just keeping this the same as the other most/* drivers
to be consistent with those for now while just fixing this current buglet.

> This way you can't have 2 variables accidentally. You will need forward 
> declarations for those two functions, but a mismatch here results in a linker 
> error rather than a runtime NULL pointer access
> 
> Best regards,
> Alexander
> 
> 
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH][next] staging: ks7010: fix null pointer dereference on priv on error exit

[Colin Ian King]

On 12/04/18 16:12, Dan Carpenter wrote:
> This isn't right.  It introduces a leak.

Yup, and I see Gustavo is working on a correct fix, I'll leave that to him.
> 
> regards,
> dan carpenter
> 
> --
> 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: wilc1000: check for kmalloc allocation failures

[Colin Ian King]

On 26/03/18 16:35, Ajay Singh wrote:
> Thanks for submitting the patch.
> 
> On Wed, 21 Mar 2018 13:03:18 -0700
> Joe Perches <j...@perches.com> wrote:
> 
>> On Wed, 2018-03-21 at 19:19 +, Colin King wrote:
>>> From: Colin Ian King <colin.k...@canonical.com>
>>>
>>> There are three kmalloc allocations that are not null checked which
>>> potentially could lead to null pointer dereference issues. Fix this
>>> by adding null pointer return checks.
>>
>> looks like all of these should be kmemdup or kstrdup
>>
>>>  
>>> @@ -951,6 +955,10 @@ static s32 handle_connect(struct wilc_vif *vif,
>>> if (conn_attr->ssid) {
>>> hif_drv->usr_conn_req.ssid = kmalloc(conn_attr->ssid_len + 1,
>>>  GFP_KERNEL);
>>> +   if (!hif_drv->usr_conn_req.ssid) {
>>> +   result = -ENOMEM;
>>> +   goto error;
>>> +   }
>>> memcpy(hif_drv->usr_conn_req.ssid,
>>>conn_attr->ssid,
>>>conn_attr->ssid_len);
> 
> With this changes the Coverity reported warning is handled correctly.
> 
> For further improvement to the patch, as Joe Perches suggested, its better
> to make use of kmemdup instead of kmalloc & memcpy. As kstrdup requires the
> source string to be NULL terminated('\0') and conn_attr->ssid might not  
> contains the '\0' terminated string. So kmemdup with length of 
> 'conn_attr->ssid_len' can be used instead.
> 
> Please include the changes by using kmemdup() for all kmalloc/memcpy in
> this patch.

The original has been included into Greg's staging repo, so I'll send a
send patch that addresses the kmemdup.

Colin
> 
> 
> 
> Regards,
> Ajay
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

NACK: [PATCH][next] staging: r8822be: fix type in guard macro __PHYDMKFREE_H__

[Colin Ian King]

On 23/03/18 17:36, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> The #define for __PHYDMKFREE_H__ is missing a character and is not
> the same as the guard check.  Defined this correctly.
> 
> Cleans up clang warning:
> warning: '__PHYDMKFREE_H__' is used as a header guard here, followed
> by #define of a different macro [-Wheader-guard]
> 
> Fixes: 9ce99b04b5b8 ("staging: r8822be: Add phydm mini driver")
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> ---
>  drivers/staging/rtlwifi/phydm/phydm_kfree.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtlwifi/phydm/phydm_kfree.h 
> b/drivers/staging/rtlwifi/phydm/phydm_kfree.h
> index 1ee60059afc1..2c6b0a48e76e 100644
> --- a/drivers/staging/rtlwifi/phydm/phydm_kfree.h
> +++ b/drivers/staging/rtlwifi/phydm/phydm_kfree.h
> @@ -24,7 +24,7 @@
>   
> */
>  
>  #ifndef __PHYDMKFREE_H__
> -#define __PHYDKFREE_H__
> +#define __PHYDKKFREE_H__
>  
>  #define KFREE_VERSION "1.0"
>  
> 
Scrub that, I'll send a V2 of this.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: lustre: lnet: convert selftest to use workqueues

[Colin Ian King]

Hi,

CoverityScan detected a couple of issues, one of which was introduced
with the following commit:

commit 6106c0f82481e686b337ee0c403821fb5c3c17ef
Author: NeilBrown 
Date:   Thu Jan 11 15:06:40 2018 +1100

CoverityScan CID#1464078 ("Uninitialized scalar variable")

I'm not sure what the expected error return for these conditions are, so
I've not fixed these as I didn't want to hazard a guess.

static int
lnet_selftest_init(void)
{
int nscheds;
int rc;

// note here rc is not initialized

int i;

lst_serial_wq = alloc_ordered_workqueue("lst_s", 0);
if (!lst_serial_wq) {
CERROR("Failed to create serial WI scheduler for LST\n");
return rc;

// garbage value in rc is returned

}
lst_init_step = LST_INIT_WI_SERIAL;

nscheds = cfs_cpt_number(lnet_cpt_table());
lst_test_wq = kvmalloc_array(nscheds, sizeof(lst_test_wq[0]),
GFP_KERNEL | __GFP_ZERO);
if (!lst_test_wq)
goto error;

// this also returns garbage in rc
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: pi433: fix missing break in switch statement.

[Colin Ian King]

On 10/11/17 16:49, Marcus Wolf wrote:
> Hi all!
> 
> Tryed to cross check...
> 
> Don't get it, sorry.
> 
> On my private version control (my SVN), where I initially developed the
> driver the break isn't missing.
> Same with my git copy of Gregs staging tree. Break is there...
> 
> Who removed it, why is it missing in Colins copy?
> 
> Am I working on a wrong version?

I was working on the latest, that got landed into linux-next. This had
picked up some modifications from Al-Viro.

Hope that clarifies things

Colin

> 
> marcus@Laptop-Wolf:~/staging/drivers/staging/pi433$ git remote show origin
> * remote origin
>   Fetch URL:
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
>   Push  URL:
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
> 
> 
> Can anybody help me?
> 
> Thanks,
> 
> Marcus
> 
> 
> Am 09.11.2017 um 19:19 schrieb Colin King:
>> From: Colin Ian King <colin.k...@canonical.com>
>>
>> The PI433_IOC_WR_RX_CFG case is missing a break and will fall through
>> to the default case and errorenously return -EINVAL. Fix this by
>> adding in missing break.
>>
>> Detected by CoverityScan, CID#1461286 ("Missing break in switch")
>>
>> Fixes: f81f0b5c9a30 ("pi433: sanitize ioctl")
>> Signed-off-by: Colin Ian King <colin.k...@canonical.com>
>> ---
>>   drivers/staging/pi433/pi433_if.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/staging/pi433/pi433_if.c
>> b/drivers/staging/pi433/pi433_if.c
>> index 3bcb59811cdf..a960fe2e7875 100644
>> --- a/drivers/staging/pi433/pi433_if.c
>> +++ b/drivers/staging/pi433/pi433_if.c
>> @@ -811,6 +811,7 @@ pi433_ioctl(struct file *filp, unsigned int cmd,
>> unsigned long arg)
>>   }
>>     mutex_unlock(>rx_lock);
>> +    break;
>>   default:
>>   retval = -EINVAL;
>>   }
>>
> -- 
> 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] fix null pointer dereferences with a null driver_adapter

[Colin Ian King]

On 08/09/17 14:52, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> The call to _rtl_dbg_trace via macro HALMAC_RT_TRACE will trigger a null
> pointer deference on a null driver_adapter.  Fix this by assigning
> driver_adapter earlier to halmac_adapter->driver_adapter before the tracing
> call so that a non-null driver_adapter is passed instead.
> 
> I should have spotted these with an earlier patch I sent, but I overlooked
> these in the rather large CoverityScan logs.
> 
> Detected by CoverityScan, CID#1454550, CID#1454554, CID#1454565,
> CID#1454591, CID#1454598 ("Explicit null dereferenced")
> 
> Fixes: 938a0447f094 ("staging: r8822be: Add code for halmac sub-driver")
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> ---
>  .../staging/rtlwifi/halmac/halmac_88xx/halmac_api_88xx.c|  4 ++--
>  .../staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c   | 13 
> +++--
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_api_88xx.c 
> b/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_api_88xx.c
> index 5f84526cb5b5..edbf6af1c8b7 100644
> --- a/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_api_88xx.c
> +++ b/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_api_88xx.c
> @@ -2901,11 +2901,11 @@ halmac_update_datapack_88xx(struct halmac_adapter 
> *halmac_adapter,
>   if (halmac_adapter->fw_version.h2c_version < 4)
>   return HALMAC_RET_FW_NO_SUPPORT;
>  
> + driver_adapter = halmac_adapter->driver_adapter;
> +
>   HALMAC_RT_TRACE(driver_adapter, HALMAC_MSG_H2C, DBG_DMESG,
>   "[TRACE]%s ==>\n", __func__);
>  
> - driver_adapter = halmac_adapter->driver_adapter;
> -
>   HALMAC_RT_TRACE(driver_adapter, HALMAC_MSG_H2C, DBG_DMESG,
>   "[TRACE]%s <==\n", __func__);
>  
> diff --git a/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c 
> b/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c
> index f33024e4d853..544f638ed3ef 100644
> --- a/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c
> +++ b/drivers/staging/rtlwifi/halmac/halmac_88xx/halmac_func_88xx.c
> @@ -1618,10 +1618,11 @@ halmac_send_h2c_set_pwr_mode_88xx(struct 
> halmac_adapter *halmac_adapter,
>   void *driver_adapter = NULL;
>   enum halmac_ret_status status = HALMAC_RET_SUCCESS;
>  
> + driver_adapter = halmac_adapter->driver_adapter;
> +
>   HALMAC_RT_TRACE(driver_adapter, HALMAC_MSG_H2C, DBG_DMESG,
>   "%s!!\n", __func__);
>  
> - driver_adapter = halmac_adapter->driver_adapter;
>   h2c_header = h2c_buff;
>   h2c_cmd = h2c_header + HALMAC_H2C_CMD_HDR_SIZE_88XX;
>  
> @@ -1713,10 +1714,11 @@ halmac_media_status_rpt_88xx(struct halmac_adapter 
> *halmac_adapter, u8 op_mode,
>   void *driver_adapter = NULL;
>   enum halmac_ret_status status = HALMAC_RET_SUCCESS;
>  
> + driver_adapter = halmac_adapter->driver_adapter;
> +
>   HALMAC_RT_TRACE(driver_adapter, HALMAC_MSG_H2C, DBG_DMESG,
>   "halmac_send_h2c_set_pwr_mode_88xx!!\n");
>  
> - driver_adapter = halmac_adapter->driver_adapter;
>   h2c_header = H2c_buff;
>   h2c_cmd = h2c_header + HALMAC_H2C_CMD_HDR_SIZE_88XX;
>  
> @@ -2143,10 +2145,11 @@ halmac_func_ctrl_ch_switch_88xx(struct halmac_adapter 
> *halmac_adapter,
>   enum halmac_cmd_process_status *process_status =
>   _adapter->halmac_state.scan_state_set.process_status;
>  
> + driver_adapter = halmac_adapter->driver_adapter;
> +
>   HALMAC_RT_TRACE(driver_adapter, HALMAC_MSG_H2C, DBG_DMESG,
>   "halmac_ctrl_ch_switch!!\n");
>  
> - driver_adapter = halmac_adapter->driver_adapter;
>   halmac_api = (struct halmac_api *)halmac_adapter->halmac_api;
>  
>   if (halmac_transition_scan_state_88xx(
> @@ -2276,15 +2279,13 @@ enum halmac_ret_status 
> halmac_send_h2c_update_bcn_parse_info_88xx(
>  {
>   u8 h2c_buff[HALMAC_H2C_CMD_SIZE_88XX] = {0};
>   u16 h2c_seq_mum = 0;
> - void *driver_adapter = NULL;
> + void *driver_adapter = halmac_adapter->driver_adapter;
>   struct halmac_h2c_header_info h2c_header_info;
>   enum halmac_ret_status status = HALMAC_RET_SUCCESS;
>  
>   HALMAC_RT_TRACE(driver_adapter, HALMAC_MSG_H2C, DBG_DMESG,
>   "%s!!\n", __func__);
>  
> - driver_adapter = halmac_adapter->driver_adapter;
> -
>   UPDATE_BEACON_PARSING_INFO_SET_FUNC_EN(h2c_buff, bcn_ie_info->func_en);
>   UPDATE_BEACON_PARSING_INFO_SET_SIZE_TH(h2c_buff, bcn_ie_info->size_th);
>   UPDATE_BEACON_PARSING_INFO_SET_TIMEOUT(h2c_buff, bcn_ie_info->timeout);
> 

Oops, I messed up the subject line, will resend V2.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: fsl-mc: fix fsl_mc_is_allocatable strcmps

[Colin Ian King]

On 16/08/17 14:37, Laurentiu Tudor wrote:
> On 08/16/2017 03:06 PM, Dan Carpenter wrote:
>> On Wed, Aug 16, 2017 at 12:44:51PM +0100, Colin King wrote:
>>> From: Colin Ian King <colin.k...@canonical.com>
>>>
>>> The previous fix removed the equal to zero comparisons by the strcmps and
>>> now the function always returns true. Fix this by adding in the missing
>>> logical negation operators.
>>>
>>> Detected by CoverityScan, CID#1452267 ("Constant expression result")
>>>
>>> Fixes: b93ad9a067e1 ("staging: fsl-mc: be consistent when checking strcmp() 
>>> return")
> 
> Thanks Colin (and Coverity) for catching this!
> 
>> Ugh...  I did review the original patch at all.  Sorry.
> 
> As a side note, funny how i got the patch description right but not the 
> actual patch. :-)
> 
>> It's better to use "== 0" because it's idiomatic.
> 
> Agree, plus this approach would be consistent with the rest of the 
> driver (except one place in drivers/staging/fsl-mc/bus/dprc-driver.c +32)

OK, I'll send a revert sometime today as that seems the sane solution.

Colin
> 
> ---
> Best Regards, Laurentiu--
> 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] Make functions rf69_set_bandwidth_intern and rf69_set_dc_cut_off_frequency_intern static

[Colin Ian King]

Oops, should have been marked as [V2] in the subject line

On 20/07/17 23:33, Colin King wrote:
> From: Colin Ian King <colin.k...@canonical.com>
> 
> The functions rf69_set_bandwidth_intern and also
> rf69_set_dc_cut_off_frequency_intern is local to the source and
> do not need to be in global scope, so make it static. Also break
> break overly wide line. Finally, remove the function declaration
> rf69_set_dc_cut_off_frequency_intern from the rf69.h header.
> 
> Cleans up sparse warning:
> symbol 'update_share_count' was not declared. Should it be static?
> 
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> ---
>  drivers/staging/pi433/rf69.c | 7 +--
>  drivers/staging/pi433/rf69.h | 1 -
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index e391ce777bc7..170e9cc59cde 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -400,7 +400,9 @@ enum lnaGain rf69_get_lna_gain(struct spi_device *spi)
>   }
>  }
>  
> -int rf69_set_dc_cut_off_frequency_intern(struct spi_device *spi ,u8 reg, 
> enum dccPercent dccPercent)
> +static int rf69_set_dc_cut_off_frequency_intern(struct spi_device *spi,
> + u8 reg,
> + enum dccPercent dccPercent)
>  {
>   switch (dccPercent) {
>   case dcc16Percent:  return WRITE_REG(reg, ( (READ_REG(reg) & 
> ~MASK_BW_DCC_FREQ) | BW_DCC_16_PERCENT) );
> @@ -433,7 +435,8 @@ int rf69_set_dc_cut_off_frequency_during_afc(struct 
> spi_device *spi, enum dccPer
>   return rf69_set_dc_cut_off_frequency_intern(spi, REG_AFCBW, dccPercent);
>  }
>  
> -int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg, enum mantisse 
> mantisse, u8 exponent)
> +static int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg,
> +  enum mantisse mantisse, u8 exponent)
>  {
>   u8 newValue;
>  
> diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
> index b81e0762032e..3fdf5d3d225b 100644
> --- a/drivers/staging/pi433/rf69.h
> +++ b/drivers/staging/pi433/rf69.h
> @@ -41,7 +41,6 @@ int rf69_set_pa_ramp(struct spi_device *spi, enum paRamp 
> paRamp);
>  int rf69_set_antenna_impedance(struct spi_device *spi, enum antennaImpedance 
> antennaImpedance);
>  int rf69_set_lna_gain(struct spi_device *spi, enum lnaGain lnaGain);
>  enum lnaGain rf69_get_lna_gain(struct spi_device *spi);
> -int rf69_set_dc_cut_off_frequency_intern(struct spi_device *spi, u8 reg, 
> enum dccPercent dccPercent);
>  int rf69_set_dc_cut_off_frequency(struct spi_device *spi, enum dccPercent 
> dccPercent);
>  int rf69_set_dc_cut_off_frequency_during_afc(struct spi_device *spi, enum 
> dccPercent dccPercent);
>  int rf69_set_bandwidth(struct spi_device *spi, enum mantisse mantisse, u8 
> exponent);
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/1][staging-next] staging: pi433: Make functions rf69_set_dc_cut_off_frequency_intern static

[Colin Ian King]

On 20/07/17 12:01, Wolf Entwicklungen wrote:
> Declare rf69_set_dc_cut_off_frequency_intern as static since it
> is used internaly only
> 
> 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
> @@ -433,7 +433,7 @@
>   return rf69_set_dc_cut_off_frequency_intern(spi, REG_AFCBW, dccPercent);
>  }
> 
> -int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg, enum mantisse 
> mantisse, u8 exponent)
> +static int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg, enum 
> mantisse mantisse, u8 exponent)
>  {
>   u8 newValue;
> diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
> --- a/drivers/staging/pi433/rf69.h
> +++ b/drivers/staging/pi433/rf69.h
> @@ -41,7 +41,6 @@
>  int rf69_set_antenna_impedance(struct spi_device *spi, enum antennaImpedance 
> antennaImpedance);
>  int rf69_set_lna_gain(struct spi_device *spi, enum lnaGain lnaGain);
>  enum lnaGain rf69_get_lna_gain(struct spi_device *spi);
> -int rf69_set_dc_cut_off_frequency_intern(struct spi_device *spi, u8 reg, 
> enum dccPercent dccPercent);
>  int rf69_set_dc_cut_off_frequency(struct spi_device *spi, enum dccPercent 
> dccPercent);
>  int rf69_set_dc_cut_off_frequency_during_afc(struct spi_device *spi, enum 
> dccPercent dccPercent);
>  int rf69_set_bandwidth(struct spi_device *spi, enum mantisse mantisse, u8 
> exponent);
> 

This is better than my original patch, so ignore my patch "staging:
pi433: Make functions rf69_set_bandwidth_intern static"

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

drivers/staging/rtl8192u

[Colin Ian King]

Hi,

Static analysis via CoverityScan picked up an issue in
cmpk_handle_query_config_rx where the following operation on an u8 is
clearly not correct:

rx_query_cfg.cfg_action = (pmsg[4] & 0x8000) >> 31;

The result of this operation is always zero. I suspect this should be:

x_query_cfg.cfg_action = (pmsg[4] & 0x80) >> 7;

..but without a datasheet I cannot check if this assumption is correct
or not. Anyhow, it is clearly a bug that needs attention.

Colin
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: unisys: ix spelling mistake of "outstanding"

[Colin Ian King]

On 29/11/16 21:02, Greg Kroah-Hartman wrote:
> On Tue, Nov 29, 2016 at 08:37:35PM +, Kershner, David A wrote:
>>
>>
>>> -Original Message-
>>> From: Colin King [mailto:colin.k...@canonical.com]
>>> Sent: Tuesday, November 29, 2016 2:07 PM
>>> To: Kershner, David A ; Greg Kroah-Hartman
>>> ; Sell, Timothy C ;
>>> Binder, David Anthony ; Arfvidson, Erik
>>> ; Frisch, Jon ; Amitoj
>>> Kaur Chawla ; *S-Par-Maintainer
>>> ; de...@driverdev.osuosl.org
>>> Cc: linux-ker...@vger.kernel.org
>>> Subject: [PATCH] staging: unisys: ix spelling mistake of "outstanding"
>>>
>>
>> Shouldn't the subjecet say "Fix" instead of fix? 
>>
>> Besides that I'm fine with the patch.
> 
> I'll go fix it up, nothing like having to do so for a patch that is
> fixing a spelling mistake...
> 
Oops, my bad. Thanks for fixing that up Greg.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] greybus: audio: ensure module is set to avoid crash on dev_err message

[Colin Ian King]

On 23/09/16 19:20, Vaibhav Agarwal wrote:
> On Fri, Sep 23, 2016 at 10:28 PM, Greg Kroah-Hartman
> <gre...@linuxfoundation.org> wrote:
>> On Fri, Sep 23, 2016 at 11:25:40AM +0100, Colin King wrote:
>>> From: Colin Ian King <colin.k...@canonical.com>
>>>
>>> Currently, if info is null, the dev_err message is dereferencing an
>>> uninitialized module pointer.  Instead, initialize module before the
>>> dev_err call to fix this issue.
>>>
>>> Found using static analysis with cppcheck:
>>> [drivers/staging/greybus/audio_topology.c:175]: (error)
>>>   Uninitialized variable: module
>>>
>>> Signed-off-by: Colin Ian King <colin.k...@canonical.com>
>>> ---
>>>  drivers/staging/greybus/audio_topology.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/staging/greybus/audio_topology.c 
>>> b/drivers/staging/greybus/audio_topology.c
>>> index 5eef536..c43a959 100644
>>> --- a/drivers/staging/greybus/audio_topology.c
>>> +++ b/drivers/staging/greybus/audio_topology.c
>>> @@ -171,6 +171,9 @@ static int gbcodec_mixer_ctl_info(struct snd_kcontrol 
>>> *kcontrol,
>>>   data = (struct gbaudio_ctl_pvt *)kcontrol->private_value;
>>>   info = (struct gb_audio_ctl_elem_info *)data->info;
>>>
>>> + module = find_gb_module(gbcodec, kcontrol->id.name);
>>> + if (!module)
>>> + return -EINVAL;
>>
>> How do you know you can get a module at this point in time, you haven't
>> looked at the type of info to know that?
>>
>> I agree that there is an issue here, but I don't think this is the
>> correct fix.  Vaibhav?
> 
> Actually, fetching module is not related to info->type. However it is
> only required in case of ENUMERATED element_type, thus used in
> specific case. Also, I think it's better to use codec->dev in the err
> message and align with other err_msg in this function.
> 
> Hi Colin, thanks for sharing this patch. Do you mind updating this
> patch including my above suggestion? Otherwise, I can share a separate
> patch.

Vaibhav, I'm currently traveling, so won't be able to check this until
next week, so if you would rather send a correct fix sooner rather me
re-sending a fix then I'm fine with that.

Thanks, Colin
> 
> --
> thanks,
> vaibhav
>>

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel