Re: [PATCH 1/1] ALSA: Fix limit of 8 PCM devices in SNDRV_CTL_IOCTL_PCM_NEXT_DEVICE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Could you split this as a separate patch?  This unsigned int -> int> change is irrelevant with the fix for 8 PCM devices limit.
Well, actually it is :-) Take a look here:
static inline int snd_pcm_next(struct snd_card *card, int device){        struct snd_pcm *pcm;
        list_for_each_entry(pcm, &snd_pcm_devices, list) {                if (pcm->card == card && pcm->device > device)                                         ^^^^^^^^^^^^^^^^^^^^                        return pcm->device;                else if (pcm->card->number > card->number)                        return -1;        }        return -1;}
When device = -1 (first step on enumeration) the expression:
  (unsigned)pcm->device > (unsigned)device
will be always 0 as (unsigned)-1 == 0xffffffff...
I wouldn't touch this at all in other case ;-)
> You cannot use SNDRV_OS_MINORS unconditionally here.> In the case DYNAMIC_MINORS=n, SNDRV_PCM_DEVICES is still valid.  Maybe> better to define SNDRV_PCM_DEVICES with ifdef, such as,> > #ifdef CONFIG_SND_DYNAMIC_MINORS> #define SNDRV_PCM_DEVICES	(SNDRV_OS_MINORS-2)> #else> #define SNDRV_PCM_DEVICES	8> #endif
To be perfectly honest I don't see the point, as in both cases the valuemay be simply wrong (I mean device not existing at all), so as for methese checks are just "safety fuses" and above is yet another #ifdef,but no problem - I will revive this constant.
> > -static struct snd_pcm *snd_pcm_search(struct snd_card *card, int> device)> > +static inline struct snd_pcm *snd_pcm_get(struct snd_card *card,> int device)> > Don't use inline.  It's good for a single-line or a pretty small> function, but not for else.  The compiler is clever enough to inline> the functions automatically if needed.
Well, I used to define functions which are called from just one place(which are usually just an extracted part of other huge function ;-) asinline, but as you wish - I wont persist on it.
> Also, any reason to rename to *_get()?  get() is usually paired with> put().
I see your point. My motivation was: this function is not used forsearching the list any more, just for getting a reference to one,defined element. Of course I can leave it as it was.
Regards
Paweł
_______________________________________________Alsa-devel mailing listAlsa-devel@xxxxxxxxxxxxxxxxxxxx://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux