TDE
/
tdelibs
7
1
Fork 0
Code Issues 16 Pull-Requests 8 Releases Wiki Aktivität
Label Meilensteine

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

Zusammengeführt
SlavekB hat 1 Commits von feat/fix-if-else-cond nach master vor 5 Jahren zusammengeführt
Diskussion 2 Commits 1 Geänderte Dateien 1
obache hat vor 5 Jahren kommentiert
Mitarbeiter

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

OSS MIDI API may be missing even if ALSA is enabled.
SlavekB hat vor 5 Jahren überprüft
SlavekB hat einen Kommentar hinterlassen
Besitzer

Please consider simplifying the code as suggested in the comment.

Please consider simplifying the code as suggested in the comment.
libtdemid/deviceman.cc
#endif
#ifdef HAVE_OSS_SUPPORT
_seqbufptr=0;
#endif
SlavekB hat vor 5 Jahren kommentiert
Besitzer

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
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 hat vor 5 Jahren kommentiert
Besitzer

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
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 hat vor 5 Jahren kommentiert
Besitzer

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 😸

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 :smile_cat:
obache hat vor 5 Jahren kommentiert
Ersteller
Mitarbeiter

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

For multiple sound systems support, is it intend to be runtime switching or build time?
obache hat vor 5 Jahren kommentiert
Ersteller
Mitarbeiter

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

For multiple sound systems support, is it intend to be runtime switching or build time?
SlavekB hat vor 5 Jahren kommentiert
Besitzer

Please give attention to my review – see comment above.

Please give attention to my review – see comment above.
obache hat vor 5 Jahren kommentiert
Ersteller
Mitarbeiter

Changed to coding style same as others.

Changed to coding style same as others.
SlavekB hat diesen Pull-Request vor 5 Jahren geschlossen
SlavekB löschte die Branch feat/fix-if-else-cond vor 5 Jahren
SlavekB hat diesen Issue vor 5 Jahren zum R14.0.6 release Meilenstein hinzugefügt
Der Pull Request wurde als dd6da7345a gemergt.
Anmelden, um an der Diskussion teilzunehmen.
Reviewer
Überprüfung anfordern
Keine Reviewer
Label
Label anwenden
Label entfernen
GE/need-info
General - need additional info from contributor PR/keep-branch
Pull request - do not delete branch after merging PR/not-ok
Pull request - need fixing PR/rfc
Pull request - request for comments PR/update-trans
Pull request - update to translation files needed PR/wip
Pull request - work in progress RS/R14.0.x
Related to R14.0.x series RS/R14.1.x
Related to R14.1.x series SL/critical
Severity level - critical SL/major
Severity level - major SL/minor
Severity level - minor SL/normal
Severity level - normal SL/regression
Severity level - regression from previous version SL/trivial
Severity level - trivial SL/wishlist
Severity level - wishlist request ST/duplicate
Status - duplicate of another issue ST/invalid
Status - invalid report ST/notourproblem
Status - not our problem ST/rejected
Status - rejected ST/wontfix
Status - won't fix ST/worksforme
Status - works for me, unable to reproduce
Kein Label GE/need-info PR/keep-branch PR/not-ok PR/rfc PR/update-trans PR/wip RS/R14.0.x RS/R14.1.x SL/critical SL/major SL/minor SL/normal SL/regression SL/trivial SL/wishlist ST/duplicate ST/invalid ST/notourproblem ST/rejected ST/wontfix ST/worksforme
Meilenstein
Meilenstein setzen
Meilenstein entfernen
Keine Einträge
Kein Meilenstein
R14.0.6 release
Zuständig
Benutzer zuweisen
Zuständige entfernen
Niemand zuständig
3 Beteiligte
Nachrichten
Fällig am

Kein Fälligkeitsdatum gesetzt.

Abhängigkeiten

Keine Abhängigkeiten gesetzt.

Referenz: TDE/tdelibs#22
Verfassen Vorschau
Laden…
Abbrechen
Speichern
Hier gibt es bis jetzt noch keinen Inhalt.
Branch „feat/fix-if-else-cond“ löschen

Das Löschen eines Branches ist permanent. Es KANN NICHT rückgängig gemacht werden. Fortfahren?

Abbrechen
Ja