Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1524410 > unrolled thread

Re: [alsa-devel] sound: bebop: strncmp length oddity

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [alsa-devel] sound: bebop: strncmp length oddity Takashi Iwai <tiwai@suse.de> - 2016-11-17 12:50 +0100
    Re: sound: bebop: strncmp length oddity Takashi Sakamoto <o-takashi@sakamocchi.jp> - 2016-11-17 21:10 +0100
      Re: sound: bebop: strncmp length oddity Nicolas Iooss <nicolas.iooss_linux@m4x.org> - 2016-11-17 22:40 +0100

#1524410 — Re: [alsa-devel] sound: bebop: strncmp length oddity

On Sat, 29 Oct 2016 23:37:00 +0200,
Joe Perches wrote:
> 
> 15 isn't the length of the string, is that really what's desired?
> 
> linux/next/sound/firewire/bebob/bebop.c
> 
> -----------------------------
> 
> static bool
> check_audiophile_booted(struct fw_unit *unit)
> {
> 	char name[24] = {0};
> 
> 	if (fw_csr_string(unit->directory, CSR_MODEL, name, sizeof(name)) < 0)
> 		return false;
> 
> 	return strncmp(name, "FW Audiophile Bootloader", 15) != 0;
> }

Indeed it looks bogus.  Even "FW...." string is already 24 letters, so
it's over name[] array size.

Sakamoto-san, could you fix it properly?


thanks,

Takashi

[toc] | [next] | [standalone]

#1524789 — Re: sound: bebop: strncmp length oddity

s/bebop/bebob/
(BeBoB is 'BridgeCo enhanced Breakout Box'.)

I'm sorry to miss this post, Joe. I was busy to prepare for Audio 
mini-conference on Linux Plumber Conference. I realized that Nicolas 
posted a patch for this issue.

[alsa-devel] [PATCH 1/1] ALSA: bebob: use the right string length in 
check_audiophile_booted()
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-October/114073.html

On Nov 17 2016 20:47, Takashi Iwai wrote:
> On Sat, 29 Oct 2016 23:37:00 +0200,
> Joe Perches wrote:
>>
>> 15 isn't the length of the string, is that really what's desired?
>>
>> linux/next/sound/firewire/bebob/bebop.c
>>
>> -----------------------------
>>
>> static bool
>> check_audiophile_booted(struct fw_unit *unit)
>> {
>> 	char name[24] = {0};
>>
>> 	if (fw_csr_string(unit->directory, CSR_MODEL, name, sizeof(name)) < 0)
>> 		return false;
>>
>> 	return strncmp(name, "FW Audiophile Bootloader", 15) != 0;
>> }
>
> Indeed it looks bogus.  Even "FW...." string is already 24 letters, so
> it's over name[] array size.
>
> Sakamoto-san, could you fix it properly?

It's OK just to compare first a few characters, while I left whole 
string for our information because model-specific information is easily 
lost.

For M-Audio's FireWire Audiophile, modalias of 
'ieee1394:ven00000D6Cmo00010060sp' hits this unit only. However the unit 
has two states relevant to loaded firmware. Initial firmware returns 'FW 
Audiophile Bootloader', while functional firmware returns 'FW 
Audiophile'. Just comparing the first 15 characters is the shortest way 
to defferentiate them. (Actually 14 characters. I guess that I did avoid 
comparison based on LWS...)

Therefore, changing comparison range is not for bug fix, just for 
readers. But I agree with these patches because the code is a bit 
confusing. I'll post a patch soon for this aim with appropriate 
information, sorry for Nicolas.


Thanks

Takashi Sakamoto

[toc] | [prev] | [next] | [standalone]

#1524828 — Re: sound: bebop: strncmp length oddity

On 17/11/16 21:07, Takashi Sakamoto wrote:
> On Nov 17 2016 20:47, Takashi Iwai wrote:
>> On Sat, 29 Oct 2016 23:37:00 +0200,
>> Joe Perches wrote:
>>>
>>> 15 isn't the length of the string, is that really what's desired?
>>>
>>> linux/next/sound/firewire/bebob/bebop.c
>>>
>>> -----------------------------
>>>
>>> static bool
>>> check_audiophile_booted(struct fw_unit *unit)
>>> {
>>>     char name[24] = {0};
>>>
>>>     if (fw_csr_string(unit->directory, CSR_MODEL, name, sizeof(name))
>>> < 0)
>>>         return false;
>>>
>>>     return strncmp(name, "FW Audiophile Bootloader", 15) != 0;
>>> }
>>
>> Indeed it looks bogus.  Even "FW...." string is already 24 letters, so
>> it's over name[] array size.
>>
>> Sakamoto-san, could you fix it properly?
> 
> It's OK just to compare first a few characters, while I left whole
> string for our information because model-specific information is easily
> lost.
> 
> For M-Audio's FireWire Audiophile, modalias of
> 'ieee1394:ven00000D6Cmo00010060sp' hits this unit only. However the unit
> has two states relevant to loaded firmware. Initial firmware returns 'FW
> Audiophile Bootloader', while functional firmware returns 'FW
> Audiophile'. Just comparing the first 15 characters is the shortest way
> to defferentiate them. (Actually 14 characters. I guess that I did avoid
> comparison based on LWS...)
> 
> Therefore, changing comparison range is not for bug fix, just for
> readers. But I agree with these patches because the code is a bit
> confusing. I'll post a patch soon for this aim with appropriate
> information, sorry for Nicolas.

All right, so long as the code is modified in a way that makes it more
consistent, I am all right. It is one patch less I need to care about :)

Nicolas

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web