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

Birleştirildi

SlavekB feat/fix-if-else-cond içindeki 1 işlemeyi master ile birleştirdi

obache yorum yaptı

Katkıcı

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

SlavekB incelendi

SlavekB bir yorum yaptı

Sahibi

Please consider simplifying the code as suggested in the comment.

libtdemid/deviceman.cc

													
					#endif

					#ifdef HAVE_OSS_SUPPORT

					    _seqbufptr=0;

					#endif

SlavekB yorum yaptı

Sahibi

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 yorum yaptı

Sahibi

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 yorum yaptı

Sahibi

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 yorum yaptı

Poster

Katkıcı

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

obache yorum yaptı

Poster

Katkıcı

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

SlavekB yorum yaptı

Sahibi

Please give attention to my review – see comment above.

obache yorum yaptı

Poster

Katkıcı

Changed to coding style same as others.

SlavekB değişiklik isteğini kapattı

SlavekB feat/fix-if-else-cond dalı silindi

SlavekB R14.0.6 release kilometre taşına ekledi

Değişiklik isteği dd6da7345a olarak birleştirildi.

Bu konuşmaya katılmak için oturum aç.

Değerlendirici yok

Atanan Kişi Yok

3 Katılımcı

Bildirimler

Bitiş Tarihi

Bitiş tarihi atanmadı.

Bağımlılıklar

Bağımlılık yok.

Referans: TDE/tdelibs#22