Codeteile auslagern. Bekomme cpplint Warnung
-
const cComponents *Components ------------------^
ist vermutlich ein Schreibfehler, will der Compiler sagen ...
-
@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:
Vielen Dank. das ändert aber nicht die Warn-Meldug in der .h Datei
void GetComponents(const cComponents *Components, cString &Text, cString &Audio, cString &Subtitle) {
...Ist das die Definition des Headers?
Wenn du die Funktion nicht komplett in die .h Datei geschrieben hast, würde ich mal die Headerdefinition mit der Implementierung exakt vergleichen.
-
Die cpplint-Warnung ist aufgrund einer älteren Empfehlung von dem Google C++ Style Guide (sie empfahlen dort dann explizit Zeiger zu verwenden, um anzuzeigen, daß der Parameter verändert wird), s.a. Confusion over cpplint warning, "Is this a non-const reference? If so, make const or use a pointer:" (falls du englisch kannst).
Du kannst dies wohl mitNOLINT(runtime/references)
deaktivieren.@NoIDE: Nein, wie kommst du drauf?
Edit: @MegaV0lt: Evtl. mal den 'fork' cpplint/cpplint benutzen?
-
@Th69 sagte in Codeteile auslagern. Bekomme cpplint Warnung:
Nein, wie kommst du drauf?
Es macht doch keinen Sinn, eine Kopie eines konstanten Zeigers zu übergeben. Aber wahrscheinlich hab ich nicht korrekt gelesen.
-
Hab mal alles im ersten Beitrag nachgetragen.
Die Funktion ist in der flat.c
Header in der flat.h
Aufruf erfolg aus der displaymenu.c (Vier mal)
-
@MegaV0lt: Welche genaue 'cpplint'-Version verwendest du denn?
@NoIDE: Nicht der Zeiger ist konstant, sondern es ist ein Zeiger auf konstante (also unveränderliche) Objekte!
const cComponents *Components
ist das gleiche wiecComponents const *Components
.
Was du meinst wärecComponents * const Components
(oderconst cComponents * const Components
). Aber selbst dann könnte man den Zeiger problemlos übergeben.PS: @MegaV0lt, ich wollte mal nachfragen, ob dir unsere Codeänderungen in if-Abfrage Vereinfachung geholfen haben - hast dich bisher nicht mehr zurückgemeldet.
-
@Th69 Ich habe das so gelassen wie ich es geschrieben habe. Funktioniert.
cpplint ist
Cpplint fork (https://github.com/cpplint/cpplint)
cpplint 1.5.5
Python 3.10.12 (main, Jun 11 2023, 05:26:28) [GCC 11.4.0]
-
Die Funktion CheckForNonConstReference ist in der aktuellen Version 1.6.1 wohl immer noch drin.
Im Python-Quellcode Zeile 101 ff. (bzw. im "Usage"-Text) steht, daß es auch noch
NOLINTNEXTLINE(...)
gibt (je nachdem wo du den Kommentar hinschreibst).
-
@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:
void GetComponents(const cComponents *Components, cString &Text, cString &Audio, cString &Subtitle) {
- Warum steht denn da eine "{" Klammer in der Header Datei?
- Steht die Implementierung nicht in der Datei
flat.c
? - BTW: Sollte das nicht eine C++ Datei sein, also
flat.cpp
?
-
@Quiche-Lorraine sagte in Codeteile auslagern. Bekomme cpplint Warnung:
Warum steht denn da eine "{" Klammer in der Header Datei?
Steht die Implementierung nicht in der Datei flat.c?
BTW: Sollte das nicht eine C++ Datei sein, also flat.cpp ?1 Copy/Paste Fehler. In der .h steht es richtig - 1 Beitrag
2 Doch. steht in der .h
3. Das muss ich noch umbenennen. Ist alles altlastZu 3: Kann ich das mischen oder muss dann alles .cpp sein?
Hier mal das GIt zur Übersicht:
https://github.com/MegaV0lt/vdr-plugin-skinflatplus
-
Holla die Waldfee!
Dein Code sieht nicht gut aus. Ein paar Tipps:
- Gebe alle C++ Dateien die Endung .cpp! Es ist, soweit ich weis, zwar nur eine Konvention, aber einige Programme interpretieren die Dateiendung!
- Lade dir mal
CppCheck
herunter und versuche zu verstehen, über was das Programm meckert. - Warum nutzt du manuelle Speicherverwaltung? Ich zähle 276
new
Anweisungen, 3malloc
Anweisungen, 24delete
Anweisungen und 2free
Anweisungen. - Achte bitte auf eine ggf. vorhandene Speicherverwaltung von einem SDK. Ich nutze z.B.
cryptopp
und da muss man stellenweise allokierte Daten mittels new übergeben und diese kümmert sich automatisch um die Freigabe. Gegebenenfalls würde ich solche Allokationen auch entsprechend dokumentieren (evt. über eine eigene spezielle new() Funktion, spezielle Typbezeichnungen,...) - Nutze verstärkt die STL, z.B.
std::string
stattchar*
. - Sei bitte vorsichtig mit der Verwendung von Pointern! Wenn du z.B.
displayvolume.h
anschaue, so verwendest du zwei Pointer. Was passiert aber wenn eine Instanz kopiert wird? Was passiert wenn das Objekt, auf welches der Pointer zeigt, ungültig (bzw. gelöscht) wird? - Lies dir diesbezüglich auch unbedingt die Rule of Five durch (https://en.cppreference.com/w/cpp/language/rule_of_three)!
- Wo ist die Funktion
GetComponents()
definiert? Ich kann diese nicht finden!
PS:
Sorry übrigens dass ich dich so zerlegt habe.
-
Vielen Dank für's drüber schauen und zerpflücken. Ich habe das Projekt übernommen als der original Autor sich zurück gezogen hatte, um es am Leben zu halten. Ich bin Anfänger in dem Bereich und bastel mehr oder weniger dran Rum. Hab schon ein paar Sachen gemacht.
Das Plugin ist schon mintestens zehn Jahre alt. Da sind viele "Altlasten" drin.Die Tipps sind sehr willkommen.
Ich werde sie auf meine Liste setzen und versuchen umzusetzen.
Dateien umbenennen kann ich versuchen. Muss aber wohl das Makefile anpassen und schauen, ob es auch in anderen Distributionen gebaut werden kann...
cppcheck bringt eigentlich keine Fehlermeldungen (--language=c++)
Das mit der Speicherverwaltung ist mir noch gar nicht aufgefallen.
Was ist der Grund Die STL zu bevorzugen? Ich versuche eher das cSting vom VDR zu verwenden.
Die beiden Pointer in displayvolume beziehen sich auf PixMaps, dei verwendet werden um den Lautstärkebalken und den Wert und eventuel das mute Symbol anzuzeigen. Da verstehe ich die Bedenken nicht so richtig.
Rule of Five werde ich mir noch anschauen...
Zu der Funktion: Die ist noch nicht im GIT, da ich sie erst im Live-Betrieb am VDR testen möchte. Mir passieren leider viel zu Oft Fehler. Bin halt kein Programmierer.
Noch mal Danke fürs schauen und kommentieren. Kommentare oder Kritik ist immer gerne gesehen. Man will ja besser werden. Und ohne Hilfe bekomm ich das auch gar nicht hin
-
@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:
Was ist der Grund Die STL zu bevorzugen? Ich versuche eher das cSting vom VDR zu verwenden.
STL steht für Standard Template Library. Das "Standard" darin sollte Grund genug sein, besser Klassen aus ihr zu benutzen, statt proprietärer Alternativen.
-
@MegaV0lt Ich würde nochmal gerne kurz auf deine Ursprüngliche Frage eingehen wollen.
@Th69 hat ja bereits geschrieben, dass die Warnung aufgrund einesr älteren Google C++ Style Guide ausgegeben wird.Ich persönlich habe auch Stellen bei mir im Code, an denen ich Referenzen als Out Parameter verwende.
Man könnte sich überlegen, warum das mal in dem Coding Guideline drinnen stand.
Das Problem bei Referenzen ist, du siehst es dem Aufruf der Funktion nicht direkt an, dass die Werte verändert werden. Du hast also irgendwie eine
GetComponents
Funktion, die nicht etwa eine Komponente zurück gibt, sondern die StringsText
,Audio
undSubtitle
verändern. Ich würde sagen, dass ist für die Funktion, rein vom Titel her, nicht erwartetes Verhalten.Man könnte, neben eine Umbennung, z.B. überlegen die Strings als Kopie zu übergeben (ok, kann teuer sein) und dann ein Tuple von Strings zurück zu geben. Dann sieht man an der Benutzung des Codes direkt, was da passiert.
Bzw. zumindest Audio und Subtitle wird ja erst vor der Funktion deklariert. Dann könnte man den String direkt in der Funktion anlegen und über den Tuple zurück geben. Dann benötigt man den Parameter gar nicht.@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:
Was ist der Grund Die STL zu bevorzugen? Ich versuche eher das cSting vom VDR zu verwenden
Die STL wird an vielen Stellen eingesetzt und ist daher super getestet. Außerdem bietet sie ein weites Spektrum an Algorithmen an, mit denen man viele Probleme elegant und einfach lösen kann.
-
Da wäre ein Name wie
InsertComponents()
wohl besser. Ich werde das gleich machen. PS: Die Funktion ist jetzt auch im GIT
-
Dann wohl eher
GetComponentsTexts
- du gibst ja keine Komponenten zurück oder fügst welche ein.
-
@Quiche-Lorraine sagte in Codeteile auslagern. Bekomme cpplint Warnung:
- Gebe alle C++ Dateien die Endung .cpp! Es ist, soweit ich weis, zwar nur eine Konvention, aber einige Programme interpretieren die Dateiendung!
Weil Programme die Endungen interpretieren, sollte man auf UNIX/Linux definitiv nicht .cpp nutzen! Das Problem ist hierbei, dass auf UNIX früher das Programm cpp existierte, welches C-Quelldateien vorverarbeitete (Präprozessor) und dann den expandierten Quellcode in .cpp Dateien ablegte, welche vom cc in .o Übersetzt wurden. Daher interpretieren noch so einige Programme .cpp nicht als C++ Sourcecode.
Es gibt eine Reihe von Dateiendungen, die sich für C++-Quelldateien etabliert haben: .C, .cc, .cxx, .cpp sind die gebräuchlichsten. .C ist von Stroustrup persönlich verwendet worden, und bereitet so ziemlich jeden auf DOS/Windows Freude. Für portable Programme ist daher .cc
oder .cxxsinnvoller, da man weder auf der einen noch auf der anderen Plattform Probleme zu erwarten hat.
-
@john-0 sagte in Codeteile auslagern. Bekomme cpplint Warnung:
Daher interpretieren noch so einige Programme .cpp nicht als C++ Sourcecode.
Ich hatte mich tatsächlich auch schon öfter mal gefragt, warum ich so viel .cc oder .cxx gesehen habe, aber nur selten .cpp. Das erklärt es!
Aus Interesse: hast du Beispiele für Programme, die .cpp anders interpretieren?
-
@john-0 sagte in Codeteile auslagern. Bekomme cpplint Warnung:
Das Problem ist hierbei, dass auf UNIX früher das Programm cpp existierte
Ups, habe gerade auf meiner Manjaro Installation das Programm cpp entdeckt.
Linux hat irgentwie lustige Befehle wie cpp, wc, tee.
-
@wob sagte in Codeteile auslagern. Bekomme cpplint Warnung:
Aus Interesse: hast du Beispiele für Programme, die .cpp anders interpretieren?
Ein heißer Kandidat dafür sind die original UNIX makes und auch ältere GNU make Versionen.
Ich habe gerade OpenIndiana installiert, und auch die neuste Version hat ein make was nur .C und .cc in den implicit rules für C++ Sourcecode kennt. Man kann entweder ein aktuelles GNU make nachinstallieren, oder man definiert explizite Regeln, um dieses „Problem“ zu umgehen. Dennoch sollte man nicht erwarten, dass diese Dateiendung korrekt out-of-the-box erkannt wird.