Fix if-else syntax for the case missing OSS support #22

Fusionado

SlavekB fusionados 1 commits de feat/fix-if-else-cond en master

obache comentado

Colaborador

OSS MIDI API may be missing even if ALSA is enabled.

SlavekB revisado

SlavekB dejó un comentario

Propietario

Please consider simplifying the code as suggested in the comment.

libtdemid/deviceman.cc

													
					#endif

					#ifdef HAVE_OSS_SUPPORT

					    _seqbufptr=0;

					#endif

SlavekB comentado

Propietario

The combination of nested #ifdef and if-else in the code looks hard to read. I propose simplification as follows:

#ifdef HAVE_ALSA_SUPPORT
  if (alsa)
  {
    ((AlsaOut *)device[default_dev])->seqbuf_clean();
    return;
  }
#endif
#ifdef HAVE_OSS_SUPPORT
  {
    _seqbufptr=0;
   return;
  }
#endif

MicheleC comentado

Propietario

a simpler version is:

#ifdef HAVE_ALSA_SUPPORT
  if (alsa)
  {
    ((AlsaOut *)device[default_dev])->seqbuf_clean();
    return;
  }
#endif
#ifdef HAVE_OSS_SUPPORT
  _seqbufptr=0;
#endif

SlavekB comentado

Propietario

Yes, of course, this can make the code even simpler. I chose the closed block also for OSS to make it clear if there will be added support for some other sound systems – for example, for Solaris 😸

obache comentado

Autor

Colaborador

For multiple sound systems support, is it intend to be runtime switching or build time?

obache comentado

Autor

Colaborador

For multiple sound systems support, is it intend to be runtime switching or build time?

SlavekB comentado

Propietario

Please give attention to my review – see comment above.

obache comentado

Autor

Colaborador

Changed to coding style same as others.

SlavekB cerró este pull request

SlavekB rama eliminada feat/fix-if-else-cond

SlavekB ha añadido esto al hito R14.0.6 release

El Pull Request se ha fusionado como dd6da7345a.

Inicie sesión para unirse a esta conversación.

No hay revisores

No asignados

3 participantes

Notificaciones

Fecha de vencimiento

Sin fecha de vencimiento.

Dependencias

No se han establecido dependencias.

Referencia: TDE/tdelibs#22