At Mon, 03 Feb 2003 07:28:41 -0500, John S. Denker <[EMAIL PROTECTED]> wrote: > > Takashi Iwai wrote: > > > (btw, i used 4 instead of sizeof(int) because the latter might not be > > always equal with 32bit (in future) :) > > Certainly assuming sizeof(int)==4 is unwise. > But using "4" isn't the optimal solution, either. > > It is not a good programming practice to throw > around magic numbers like "4" without documenting > where they came from. In this case it would be > better to use > snd_pcm_format_size(format, nsamples)
well, unfortunately, it's not a constant, so we cannot use it as the initializer. i believe, in this case, giving a comment like /* 8192 frames */ would suffice, because it's obvious that the hardware supports only 32bit samples. surely it was my fault, only putting a magic number there. btw, sizeof() is not a general solution for this, too, because the buffer size is not always aligned to any values of architecture (e.g. 24bit/3bytes format). > At Sun, 2 Feb 2003 16:37:46 -0500, > Paul Davis wrote: > > >i don't know how we ended up with such incorrect values for the > >period/buffer size limits, > > That is a very important question! > It is by asking such questions that one grows > as a programmer. > > To be explicit: whenever you see a bug, don't just > ask how to fix this instance of the bug; ask what > it would take to make sure no bug of this ilk ever > occurs again. > > In this case: > a) It would help to have some comments in the code > saying where the constants are coming from, so that > misconceptions can be more easily spotted. > b) It would help to use correctly computed constants > such as > snd_pcm_format_size(format, nsamples) > rather than magic numbers such as "4". There are > far too many magic numbers in the ALSA code at > the moment. > c) When applying the sizeof() operator, don't > apply it to general-purpose types such as int, but > rather apply it to an object that is provably > of the type you care about, in this case something > like sizeof(s32_le_t). Now there isn't AFAIK any > typedef for s32_le_t, but you could define one > (carefully!). If you don't have such a typedef, > don't assume sizeof(int) is synonymous with the > sizeof(the_thing_you_care_about). for kernel codes, you can use sizeof(s32). Takashi ------------------------------------------------------- This SF.NET email is sponsored by: SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See! http://www.vasoftware.com _______________________________________________ Alsa-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/alsa-devel
