[alsa-devel] endianness problems in fireworks/bebob_maudio
Hi,
sparse complains:
sound/firewire/fireworks/fireworks_transaction.c:127:18: warning: restricted __be32 degrades to integer
t = (struct snd_efw_transaction *)data; length = min_t(size_t, t->length * sizeof(t->length), length);
't->length' is still a big-endian value. This means that the driver ends up always using 'length'.
sound/firewire/bebob/bebob_maudio.c:100:17: warning: incorrect type in initializer (different base types) sound/firewire/bebob/bebob_maudio.c:100:17: expected restricted __be32 sound/firewire/bebob/bebob_maudio.c:100:17: got int
__be32 cues[3] = { MAUDIO_BOOTLOADER_CUE1, MAUDIO_BOOTLOADER_CUE2, MAUDIO_BOOTLOADER_CUE3 };
rcode = fw_run_transaction(device->card, TCODE_WRITE_BLOCK_REQUEST, device->node_id, device->generation, device->max_speed, BEBOB_ADDR_REG_REQ, cues, sizeof(cues));
The three MAUDIO_BOOTLOADER_CUEx values will end up as a different byte sequence on big-endian machines. The simplest way to have these twelve bytes unchanged on the bus is to have a twelve-byte array in the driver.
Regards, Clemens
Hi Clemens,
On Dec 8 2014 06:24, Clemens Ladisch wrote:
sound/firewire/fireworks/fireworks_transaction.c:127:18: warning: restricted __be32 degrades to integer
t = (struct snd_efw_transaction *)data; length = min_t(size_t, t->length * sizeof(t->length), length);'t->length' is still a big-endian value. This means that the driver ends up always using 'length'.
Exactly. I agree they should be fixed.
sound/firewire/bebob/bebob_maudio.c:100:17: warning: incorrect type in initializer (different base types) sound/firewire/bebob/bebob_maudio.c:100:17: expected restricted __be32 sound/firewire/bebob/bebob_maudio.c:100:17: got int
__be32 cues[3] = { MAUDIO_BOOTLOADER_CUE1, MAUDIO_BOOTLOADER_CUE2, MAUDIO_BOOTLOADER_CUE3 }; rcode = fw_run_transaction(device->card, TCODE_WRITE_BLOCK_REQUEST, device->node_id, device->generation, device->max_speed, BEBOB_ADDR_REG_REQ, cues, sizeof(cues));The three MAUDIO_BOOTLOADER_CUEx values will end up as a different byte sequence on big-endian machines. The simplest way to have these twelve bytes unchanged on the bus is to have a twelve-byte array in the driver.
I'm too lazy for these codes. Yes, they should be fixed as long as we assume this driver is used in big-endian machine.
For my information, Would you tell me the way to generate these sparse report? I've been working with sparse for my patchset and never see such reports, furthermore never receive such reports from kernel build bot.
Thanks
Takashi Sakamoto o-takashi@sakamocchi.jp
On Dec 08 2014 17:09, Clemens Ladisch wrote:
Takashi Sakamoto wrote:
For my information, Would you tell me the way to generate these sparse report?
make C=1 CF=-D__CHECK_ENDIAN__
I always check with 'make C=2' without the 'CF' flag. Firewire drivers handle big-endian data, so the flag is nice.
Thanks
Takashi Sakamoto o-takashi@sakamocchi.jp
Although the 't->length' is a big-endian value, it's used without any conversion. This means that the driver always uses 'length' parameter.
Fixes: 555e8a8f7f14("ALSA: fireworks: Add command/response functionality into hwdep interface") Reported-by: Clemens Ladisch clemens@ladisch.de Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/fireworks/fireworks_transaction.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/firewire/fireworks/fireworks_transaction.c b/sound/firewire/fireworks/fireworks_transaction.c index 255dabc..2a85e42 100644 --- a/sound/firewire/fireworks/fireworks_transaction.c +++ b/sound/firewire/fireworks/fireworks_transaction.c @@ -124,7 +124,7 @@ copy_resp_to_buf(struct snd_efw *efw, void *data, size_t length, int *rcode) spin_lock_irq(&efw->lock);
t = (struct snd_efw_transaction *)data; - length = min_t(size_t, t->length * sizeof(t->length), length); + length = min_t(size_t, be32_to_cpu(t->length) * sizeof(u32), length);
if (efw->push_ptr < efw->pull_ptr) capacity = (unsigned int)(efw->pull_ptr - efw->push_ptr);
At Thu, 8 Jan 2015 00:31:16 +0900, Takashi Sakamoto wrote:
Although the 't->length' is a big-endian value, it's used without any conversion. This means that the driver always uses 'length' parameter.
Fixes: 555e8a8f7f14("ALSA: fireworks: Add command/response functionality into hwdep interface") Reported-by: Clemens Ladisch clemens@ladisch.de Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
Applied, thanks.
Takashi
sound/firewire/fireworks/fireworks_transaction.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/firewire/fireworks/fireworks_transaction.c b/sound/firewire/fireworks/fireworks_transaction.c index 255dabc..2a85e42 100644 --- a/sound/firewire/fireworks/fireworks_transaction.c +++ b/sound/firewire/fireworks/fireworks_transaction.c @@ -124,7 +124,7 @@ copy_resp_to_buf(struct snd_efw *efw, void *data, size_t length, int *rcode) spin_lock_irq(&efw->lock);
t = (struct snd_efw_transaction *)data;
- length = min_t(size_t, t->length * sizeof(t->length), length);
length = min_t(size_t, be32_to_cpu(t->length) * sizeof(u32), length);
if (efw->push_ptr < efw->pull_ptr) capacity = (unsigned int)(efw->pull_ptr - efw->push_ptr);
-- 2.1.0
participants (3)
-
Clemens Ladisch -
Takashi Iwai -
Takashi Sakamoto