Messages in this thread | | | Date | Thu, 17 Oct 2013 08:13:32 -0300 | Subject | Re: [PATCH] sound: pci: emu10k1: code refactoring and casting removal | From | Geyslan Gregório Bem <> |
| |
2013/10/17 Takashi Iwai <tiwai@suse.de>: > At Wed, 16 Oct 2013 19:11:21 -0300, > Geyslan G. Bem wrote: >> >> Partially restructures _snd_emu10k1_audigy_init_efx() and >> _snd_emu10k1_init_efx() functions. >> >> Removes useless casting (void *) from value returned by kcalloc; >> see Documentation/CodingStyle, Chap 14. >> >> Signed-off-by: Geyslan G. Bem <geyslan@gmail.com> >> --- >> sound/pci/emu10k1/emufx.c | 84 ++++++++++++++++++++++++++--------------------- >> 1 file changed, 46 insertions(+), 38 deletions(-) >> >> diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c >> index 0275209..afd4691 100644 >> --- a/sound/pci/emu10k1/emufx.c >> +++ b/sound/pci/emu10k1/emufx.c >> @@ -1182,15 +1182,20 @@ static int _snd_emu10k1_audigy_init_efx(struct snd_emu10k1 *emu) >> u32 *gpr_map; >> mm_segment_t seg; >> >> - if ((icode = kzalloc(sizeof(*icode), GFP_KERNEL)) == NULL || >> - (icode->gpr_map = (u_int32_t __user *) >> - kcalloc(512 + 256 + 256 + 2 * 1024, sizeof(u_int32_t), >> - GFP_KERNEL)) == NULL || >> - (controls = kcalloc(SND_EMU10K1_GPR_CONTROLS, >> - sizeof(*controls), GFP_KERNEL)) == NULL) { >> - err = -ENOMEM; >> - goto __err; >> - } >> + err = -ENOMEM; >> + icode = kzalloc(sizeof(*icode), GFP_KERNEL); >> + if (!icode) >> + return err; >> + >> + icode->gpr_map = (__user) kcalloc(512 + 256 + 256 + 2 * 1024, >> + sizeof(u_int32_t), GFP_KERNEL); >> + if (!icode->gpr_map) >> + goto __err_gpr; >> + controls = kcalloc(SND_EMU10K1_GPR_CONTROLS, >> + sizeof(*controls), GFP_KERNEL); >> + if (!controls) >> + goto __err_ctrls; >> + >> gpr_map = (u32 __force *)icode->gpr_map; >> >> icode->tram_data_map = icode->gpr_map + 512; >> @@ -1741,12 +1746,12 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_GPR(var), A_GPR(vol), A_EXTIN(input)) >> emu->support_tlv = 0; /* clear again */ >> snd_leave_user(seg); >> >> - __err: >> +__err: >> kfree(controls); >> - if (icode != NULL) { >> - kfree((void __force *)icode->gpr_map); >> - kfree(icode); >> - } >> +__err_ctrls: >> + kfree((void __force *)icode->gpr_map); >> +__err_gpr: >> + kfree(icode); >> return err; >> } >> >> @@ -1813,18 +1818,26 @@ static int _snd_emu10k1_init_efx(struct snd_emu10k1 *emu) >> u32 *gpr_map; >> mm_segment_t seg; >> >> - if ((icode = kzalloc(sizeof(*icode), GFP_KERNEL)) == NULL) >> - return -ENOMEM; >> - if ((icode->gpr_map = (u_int32_t __user *) >> - kcalloc(256 + 160 + 160 + 2 * 512, sizeof(u_int32_t), >> - GFP_KERNEL)) == NULL || >> - (controls = kcalloc(SND_EMU10K1_GPR_CONTROLS, >> - sizeof(struct snd_emu10k1_fx8010_control_gpr), >> - GFP_KERNEL)) == NULL || >> - (ipcm = kzalloc(sizeof(*ipcm), GFP_KERNEL)) == NULL) { >> - err = -ENOMEM; >> - goto __err; >> - } >> + err = -ENOMEM; >> + icode = kzalloc(sizeof(*icode), GFP_KERNEL); >> + if (!icode) >> + return err; >> + >> + icode->gpr_map = (__user) kcalloc(256 + 160 + 160 + 2 * 512, >> + sizeof(u_int32_t), GFP_KERNEL); >> + if (!icode->gpr_map) >> + goto __err_gpr; >> + >> + controls = kcalloc(SND_EMU10K1_GPR_CONTROLS, >> + sizeof(struct snd_emu10k1_fx8010_control_gpr), >> + GFP_KERNEL); >> + if (!controls) >> + goto __err_ctrls; >> + >> + ipcm = kzalloc(sizeof(*ipcm), GFP_KERNEL); >> + if (!ipcm) >> + goto __err_ipcm; >> + >> gpr_map = (u32 __force *)icode->gpr_map; >> >> icode->tram_data_map = icode->gpr_map + 256; >> @@ -2335,14 +2348,8 @@ static int _snd_emu10k1_init_efx(struct snd_emu10k1 *emu) >> for (z = 0; z < 16; z++) >> OP(icode, &ptr, iACC3, FXBUS2(z), C_00000000, C_00000000, EXTIN(z)); >> } >> - >> >> - if (gpr > tmp) { >> - snd_BUG(); >> - err = -EIO; >> - goto __err; >> - } >> - if (i > SND_EMU10K1_GPR_CONTROLS) { >> + if (gpr > tmp || i > SND_EMU10K1_GPR_CONTROLS) { >> snd_BUG(); > > In that way, you can't distinguish which condition triggered the bug > (it could be shown in WARN() called in snd_BUG() in the original > version), so this is a functional change. Avoid it in a clean up > patch. > > > thanks, > > Takashi
Takashi, I actually thought that that change was a cleanup. :) But ok, now I see the reason (tracing) What about using this instead snd_BUG?
snd_printd(KERN_WARN "BUG?\ngpr: %d, i: %d", gpr, i);
It prints file, line and we can put the data. I really want to reduce repeated code.
Regards, Geyslan
| |