Codeteile auslagern. Bekomme cpplint Warnung

*john 0

Ok, ich habe mir die Sourcen des Hauptprojektes tvdr.de angeschaut. Da herrscht ebenfalls dieses Wirrwarr. Persönlich sehe ich da keine große Chance ein Plugin auf saubere C++ umzustellen, wenn das Hauptprojekt schon so wirr ist.

@MegaV0lt Mein Beileid, aber da hast Du Dir einen sehr hässlichen Brocken für ein Anfängerprojekt ans Land gezogen.

Schlangenmensch

@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:

Der Code wird aber mit gcc übersetzt als 'c11' Code übersetzt.

Irgendwie erscheint mir das komisch. Wenn du das als C-Code übersetzt kann das mit dem class Keyword schon nicht funktionieren.
In C Code kannst du auch keine Smartpointer einsetzen.

Deine Fehlermeldung kommt daher, dass du die Funktionsdefinition in der Source Datei geändert hast, aber nicht die Deklaration im Header.

Du kannst mit C Compilern keinen C++ Code kompilieren. C++ unterstützt einige C-Feature, aber da ist Vorsicht geboten, nicht alles, was man aus C kennt, bedeutet in C++ dasselbe. Du musst schon genau wissen, welche Sprache mit welchem Standard du verwenden willst

MegaV0lt

@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:

-std=c++11

Das sollte doch für c++ sein?
Ich habe das gemacht, weil ich das Skin-Plugin selber nutze und nicht möchte, dass es irgendwann obsolet wird.

Zum Glück habe ich bis jetzt alles so weit mit Hilfen zum laufen gebracht und auch einige Sachen neu eingebaut und "optimiert". Beispiel Das Laden der Bilder für die Darsteller wurde immer zwei mal durchlaufen, um festzustellen, ob der Inhalt mit oder ohne Scrollbalken angezeigt wird. Hab es so umgeschrieben, dass angenommen wird, dass Scrollbalken benötigt werden und falls nicht die zweite Schleife auf zwischengespeicherte Werte zugreift...

Leider habe ich auch nicht all zu viel Zeit um mich tiefer einzuarbeiten, aber ich bin trotzdem einigermaßen zufrieden, dass es stabil läuft und macht was es soll.

Vielen Dank noch mal für die vielen Tipps und Vorschläge

firefly

@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:

Der Code wird aber mit gcc übersetzt als 'c11' Code übersetzt.

@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:

-std=c++11

c11 != c++11. Daher die Verwirrung bei uns ob du nun C++ oder C machst.

DocShoe

@MegaV0lt

Ja, das zieht sich dann quer durch die Vererbungshierachie. Die Umstellung auf shared_ptr erfordert dann, dass die Signaturen der Basisklasse angepasst werden. Das kann schnell eskalieren

MegaV0lt

@DocShoe sagte in Codeteile auslagern. Bekomme cpplint Warnung:

Epgsearch_lastconflictinfo_v1_0 *serviceData = new Epgsearch_lastconflictinfo_v1_0;
if( service_data )
{
}
delete service_data;

// ist in mehrerer Hinsicht falsch:
// 1) new gibt im Fehlerfall keinen nullptr zurück, sondern wirft eine bad_alloc exception. Die Überprüfung des Rückgabewertes ist also sinnlos
// 2) warum wird service_data überhaupt auf dem Heap erzeugt und nicht auf dem Stack? Nur weil iwo ein Zeiger als Parameter erwartet wird heißt
// das nicht, dass das Objekt auf dem Heap erzeugt werden muss.
Epgsearch_lastconflictinfo_v1_0 serviceData;
serviceData.nextConflict = 0;
serviceData.relevantConflicts = 0;
serviceData.totalConflicts = 0;
p->Service("Epgsearch-lastconflictinfo-v1.0", &serviceData);
if (serviceData.relevantConflicts > 0)
{
numConflicts = serviceData.relevantConflicts;

Das is auch so eine Altlast. Kann ich da das new einfach weglassen oder muss das dann ganz anders gemacht werden?

Th69

Der dort stehende Code ist doch schon die korrigierte Version (oder siehst du da noch ein new?).

Es gäbe nur einen Grund, dort serviceData im Freispeicher (Heap) zu allozieren, nämlich wenn die Service-Funktion den übergebenen Zeiger langfristig speichern würde (und dann außerhalb der aufrufenden Funktion noch mal darauf zugegriffen würde) - aber dann dürfte dort auch kein delete serviceData (im Code von @DocShoe steht ja fälschlicherweise service_data ;- ) stehen.

DocShoe

@Th69 sagte in Codeteile auslagern. Bekomme cpplint Warnung:

...
(im Code von @DocShoe steht ja fälschlicherweise service_data ;- )
...

Waaaas? Kann gar nicht sein!
Ergänzung zum Beitrag:
3) delete service_data versucht Speicher freizugeben, der unter dem Variablennamen überhaupt nicht allokiert wurde.

MegaV0lt

@MegaV0lt sagte in Codeteile auslagern. Bekomme cpplint Warnung:

Epgsearch_lastconflictinfo_v1_0 *serviceData = new Epgsearch_lastconflictinfo_v1_0;

~~Das new ist noch da (1. Zeile).~~
Edit: Kommando zurück... Hab den korrigierten Code nicht gesehen... Teste das gleich mal...
Edit2: Klappt super! Vielen Dank und sorry, dass ich es nicht gleich gesehen habe

MegaV0lt

Dachte mir ich mach das bei cIndexFile auch so. Leider klappt das nicht:

g++ -Wall -g -O3 -Wall -Werror=overloaded-virtual -Wno-parentheses -fPIC -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -std=c++11 -c -DPLUGIN_NAME_I18N='"skinflatplus"' -DVDRLOGO=\"vdrlogo_yavdr\" -DWIDGETFOLDER='"/usr/lib/vdr/plugins/skinflatplus/widgets"' -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/GraphicsMagick -o displayvolume.o displayvolume.c
displayreplay.c: In member function ‘void cFlatDisplayReplay::UpdateInfo()’:
displayreplay.c:335:29: error: cannot convert ‘cIndexFile’ to ‘cIndexFile*’ in assignment
  335 |             index = /*new*/ cIndexFile(recording->FileName(), false, recording->IsPesRecording());
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                             |
      |                             cIndexFile
make[1]: *** [Makefile:102: displayreplay.o] Fehler 1

Der Code sieht so aus:

            cMarks marks;
            // From skinElchiHD - Avoid triggering index generation for recordings with empty/missing index
            bool hasMarks = false;
            cIndexFile *index = NULL;
            if (Recording->NumFrames() > 0) {
                hasMarks =
                    marks.Load(Recording->FileName(), Recording->FramesPerSecond(), Recording->IsPesRecording()) &&
                    marks.Count();
                index = new cIndexFile(Recording->FileName(), false, Recording->IsPesRecording());
            }

Später wird dann auf index so zugegriffen:

            if (hasMarks && index) {
                uint16_t FileNumber;
                off_t FileOffset;

                bool cutin = true;
                int32_t position {0};
                cMark *mark = marks.First();
                while (mark) {
                    position = mark->Position();
                    index->Get(position, &FileNumber, &FileOffset);

Schlangenmensch

@MegaV0lt Wenn du ein C++ Projekt pflegen willst, solltest du dir dringend Literatur anschaffen und lesen, oder zumindest mal irgendwo ein online Tutorial machen, so wird das echt mühselig.

cIndexFile *index = NULL;

index Ist ein Pointer. new gibt immer Pointer zurück.

Wenn cIndexFile einen Ctor ohne Parameter anbietet, kannst du das versuchen:

 bool hasMarks = false;
 cIndexFile index;
 if (Recording->NumFrames() > 0) {
     hasMarks = marks.Load(
                   Recording->FileName(), 
                   Recording->FramesPerSecond(),
                   Recording->IsPesRecording()) &&
                marks.Count();
     index = cIndexFile(Recording->FileName(), false, Recording->IsPesRecording());
   }

Den Zugriff auf Memberfunktionen musst du dann aber auch ändern:

 index.Get(position, &FileNumber, &FileOffset);

MegaV0lt

Vielen Dank! Ctor geht nicht:

class cIndexFile {
private:
  int f;
  cString fileName;
  int size, last;
  tIndexTs *index;
  bool isPesRecording;
  cResumeFile resumeFile;
  cIndexFileGenerator *indexFileGenerator;
  cMutex mutex;
  void ConvertFromPes(tIndexTs *IndexTs, int Count);
  void ConvertToPes(tIndexTs *IndexTs, int Count);
  bool CatchUp(int Index = -1);
public:
  cIndexFile(const char *FileName, bool Record, bool IsPesRecording = false, bool PauseLive = false, bool Update = false);
  ~cIndexFile();
  bool Ok(void) { return index != NULL; }
  bool Write(bool Independent, uint16_t FileNumber, off_t FileOffset);
  bool Get(int Index, uint16_t *FileNumber, off_t *FileOffset, bool *Independent = NULL, int *Length = NULL);
  int GetNextIFrame(int Index, bool Forward, uint16_t *FileNumber = NULL, off_t *FileOffset = NULL, int *Length = NULL);
  int GetClosestIFrame(int Index);
       ///< Returns the index of the I-frame that is closest to the given Index (or Index itself,
       ///< if it already points to an I-frame). Index may be any value, even outside the current
       ///< range of frame indexes.
       ///< If there is no actual index data available, 0 is returned.
  int Get(uint16_t FileNumber, off_t FileOffset);
  int Last(void) { CatchUp(); return last; }
       ///< Returns the index of the last entry in this file, or -1 if the file is empty.
  int GetResume(void) { return resumeFile.Read(); }
  bool StoreResume(int Index) { return resumeFile.Save(Index); }
  bool IsStillRecording(void);
  void Delete(void);
  static int GetLength(const char *FileName, bool IsPesRecording = false);
       ///< Calculates the recording length (number of frames) without actually reading the index file.
       ///< Returns -1 in case of error.
  static cString IndexFileName(const char *FileName, bool IsPesRecording);
  };

DocShoe

Generell gilt: Die Konstruktion eines Objekts erfordert immer einen passender Konstruktoraufruf. Die Parameter, die du bei der Konstruktion mit new übergibst, musst du auch bei der Konstruktion auf dem Stack angeben:

// aus
cIndexFile* index = new cIndexFile(Recording->FileName(), false, Recording->IsPesRecording());

// wird
cIndexFile index(Recording->FileName(), false, Recording->IsPesRecording());

Ansonsten muss ich Schlangenmensch zustimmen, du solltest dir einen Kenntnisstand erarbeiten, mit dem du zumindest einfache Probleme selbst lösen kannst. Für jeden Compilerfehler einen Beitrag zu schreiben dauert ewig und vergrault Regs, die anfangs noch hilfsbereit sind.