Memoryleak komme nicht weiter



  • Die Berechnung von _HeaderSize (BTW: nicht erlaubter Name, da Unterstrich + Großbuchstabe) sieht mir zweifelhaft aus. Warum berechnest du das überhaupt von Hand und benutzt nicht std::string?



  • hier wirds es aufgerufen:

    void HttpResponse::send(Connection* curconnection,const char* data, size_t datalen){
      _HeadlineLen=snprintf(_Headline,512,"%s %s\r\n",_Version,_State);
      setData("Content-Length",datalen);
      const char *header=getHeader();
      curconnection->addSendQueue(header,getHeaderSize());
      if(datalen!=0)
        curconnection->addSendQueue(data,datalen);
    }
    

    mit unterstrich makiere ich zusätzlich private variablen



  • das funkioniert erstmal

    std::stringstream hstream;
      hstream << _Headline;
      size_t pos=_HeadlineLen;
      for(HeaderData *curdat=_firstHeaderData; curdat; curdat=curdat->_nextHeaderData){
        hstream << curdat->_Key << ": " << curdat->_Value << "\r\n";
      }
      hstream << "\r\n";
      std::string hdat=hstream.str();
      _Header=new char[hdat.length()];
      std::copy(hdat.c_str(),hdat.c_str()+hdat.length(),_Header);
      std::cerr << _Header;
      return _Header;
    


  • Wie bereits gesagt, std::string und unique_ptr sind die Lösungen.



  • 🙄

    Du gewinnst dadurch aber nichts.. das ist dir schon klar, oder? Ein delete[] fehlt und das fehlt nach deiner Änderung immer noch. Oder eben: std::string zurückgeben!



  • Tuxist schrieb:

    das funkioniert erstmal

    std::stringstream hstream;
      hstream << _Headline;
      size_t pos=_HeadlineLen;
      for(HeaderData *curdat=_firstHeaderData; curdat; curdat=curdat->_nextHeaderData){
        hstream << curdat->_Key << ": " << curdat->_Value << "\r\n";
      }
      hstream << "\r\n";
      std::string hdat=hstream.str();
      _Header=new char[hdat.length()];
      std::copy(hdat.c_str(),hdat.c_str()+hdat.length(),_Header);
      std::cerr << _Header;
      return _Header;
    

    sehe ich das richtig, dass es sich beim Header einfach um eine Ansammlung von chars handelt?
    Wenn ja: dann nimm einfach std::string _Header; und dort kopierst du dann das Ergebnis rein.
    Wenn du auch 0-Zeichen brauchst, dann verwende statt einem String ein Byte-Array, am einfachsten std::vector<char> _Header; in die Klassendefinition einfügen.

    Man kann natürlich auch mit new und delete fehlerfrei arbeiten, es ist aber viel schwieriger und man muss sehr aufpassen, auch immer delete aufzurufen. Daher am besten soweit möglich darauf verzichten. Und dein Anwendungsfall ist problemlos ohne new und delete machbar.



  • std::string kann problemlos nullbytes speichern.
    Das Problem an manuellem delete ist vor allem die fehlende Exceptionsicherheit.



  • Also weisst Du jetzt die (Maximal-)Länge des Strings vorher oder nicht?

    _HeaderSize=((_HeaderDataSize+_HeadlineLen)+((4*_Elements)+3));
    _Header=new char[(_HeaderSize+1)];
    

    Reicht das um da mehrfach

    "%s: %s\r\n",curdat->_Key,curdat->_Value
    

    rein zu drucken? Das erscheint mir schon fragwürdig.

    Definitiv hast Du aber in Z. 12

    snprintf(_Header+pos,_HeaderSize,"\r\n");
    

    nur noch Headersize-pos+1 Bytes Platz (inkl. '\0').

    Mit ein bisschen Sorgfalt sollte das eigentlich fliegen.



  • die informationen greife ich vorher ab:

    HttpHeader::HeaderData *HttpHeader::setData(const char* key, const char* value){
      for(HeaderData *curdat=_firstHeaderData; curdat; curdat=curdat->_nextHeaderData){
        if(strncmp(curdat->_Key,key,curdat->_Keylen)==0){
           return setData(key,value,curdat);
        }
      }
      if(!_firstHeaderData){
        _firstHeaderData=new HeaderData(key,value);
        _lastHeaderData=_firstHeaderData;
      }else{
        _lastHeaderData->_nextHeaderData=new HeaderData(key,value);
        _lastHeaderData=_lastHeaderData->_nextHeaderData;
      }
      _HeaderDataSize+=(_lastHeaderData->_Keylen+_lastHeaderData->_Valuelen);
      _Elements++;
      return _lastHeaderData;
    }
    

    oder verändere sie entsprechend

    void HttpHeader::deldata(const char* key){
      HeaderData *prevdat=NULL;
      for(HeaderData *curdat=_firstHeaderData; curdat; curdat=curdat->_nextHeaderData){
        if(strncmp(curdat->_Key,key,curdat->_Keylen)==0){
          if(prevdat){
            prevdat->_nextHeaderData=curdat->_nextHeaderData;
            if(_lastHeaderData==curdat)
              _lastHeaderData=prevdat;
          }else{
            _firstHeaderData=curdat->_nextHeaderData;
            if(_lastHeaderData==curdat)
              _lastHeaderData=_firstHeaderData;
          }
          _Elements--;
          _HeaderDataSize-=(curdat->_Keylen+curdat->_Valuelen);
          curdat->_nextHeaderData=NULL;
          delete curdat;
          return;
        }
        prevdat=curdat;
      }
    }
    
    HttpHeader::HeaderData *HttpHeader::setData(const char* key, const char* value,
    					    HttpHeader::HeaderData *pos){
      if(pos){
        _HeaderDataSize-=(pos->_Keylen+pos->_Valuelen);
        delete[] pos->_Key;
        delete[] pos->_Value;
        pos->_Keylen=strlen(key);
        pos->_Valuelen=strlen(value);
        pos->_Key=new char[pos->_Keylen+1];
        pos->_Value=new char[pos->_Valuelen+1];
        std::copy(key,key+(pos->_Keylen+1),pos->_Key);
        std::copy(value,value+(pos->_Valuelen+1),pos->_Value);
        _HeaderDataSize+=(pos->_Keylen+pos->_Valuelen);
        return pos;
      }else{
        setData(key,value);
      }
      return pos;
    }
    


  • Und was erwartet du jetzt?



  • die berrechung der größe vom char array



  • *hust*
    Warum benutzt du immer noch kein std::string?



  • weil ich alles später char blöcke zerteilen muss für sento und recvfrom

    ConnectionData *Connection::addSendQueue(const char*data,size_t datasize){
      size_t written=0;
      for(size_t cursize=datasize; cursize>0; cursize=datasize-written){
        if(cursize>BLOCKSIZE){
          cursize=BLOCKSIZE;
        }
        if(!_SendDataFirst){
          _SendDataFirst= new ConnectionData(data+written,cursize);
          _SendDataLast=_SendDataFirst;
        }else{
          _SendDataLast->_nextConnectionData=new ConnectionData(data+written,cursize);
          _SendDataLast=_SendDataLast->_nextConnectionData;
        }
        written+=cursize;
      }
      _SendDataSize+=written;
      return _SendDataLast;
    }
    
    ConnectionData *Connection::resizeSendQueue(size_t size){
      return _resizeQueue(&_SendDataFirst,&_SendDataLast,&_SendDataSize,size);
    }
    
    ConnectionData *Connection::_resizeQueue(ConnectionData** firstdata, ConnectionData** lastdata,
    					 size_t *qsize, size_t size){
      ConnectionData *firstdat=*firstdata;
      while(firstdata && size!=0){
        size_t delsize=0;
        if(size<=firstdat->getDataSize()){
           delsize=firstdat->getDataSize();
           ConnectionData *deldat=*firstdata;
           *firstdata=firstdat->_nextConnectionData;
           if(deldat==*lastdata)
             *lastdata=*firstdata;
           deldat->_nextConnectionData=NULL;
           delete deldat;
        }else{
           delsize=size;
           firstdat->_DataSize-=size;
        }
        size-=delsize;
        *qsize-=delsize;
      }
      return firstdat;
    }
    


  • Wie meinen? Von einem std::string kannst du dir mir data() oder c_str() einen zeiger auf die Daten holen. Du kannst komplett das gleiche machen wie dein new/delete-Gewurschtel, nur gibt es automatisches Aufräumen und Exceptionsicherheit gratis dazu. Wer manuell Speicher verwaltet, hat schon verloren und wird von den Javamännern ausgelacht. Siehst du ja gerade selbst.



  • gebe ich dir ja prinzipiel recht in dem fall soll es die bibliothek machen. Damit man es späteer nicht mehr selber machen muss. Recv und Send arbeiten mit festen Blockgrößen anders gar nicht realisierbar also brauche ich einen block basierten memorypool den ich schon gebaut habe. Bei aufbau habe ein bißchen mich an tinyxml2 orentiert.



  • Selbst wenn du einer Bibliothek char-Blöcke übergeben müsstest - und ich sehe nicht, warum das hier der Fall sein sollte, feste Blockgrößen sind schon mal kein Grund -, kannst du immer noch std::string zum Zusammensetzen der Daten nutzen und den fertigen String dann in char-Blöcke kopieren. Du könntest also fast in deinem gesamten Code auf dieses new-delete-Gefrickel mit manuell berechneten Arraygrößen verzichten. Versuchs doch wenigstens mal!



  • das hat jetzt funktioniert keine warum beim vorberrechnen die fehler gemacht hat.

    size_t HttpHeader::getHeaderSize(){
      size_t hsize=0;
      for(HeaderData *curdat=_firstHeaderData; curdat; curdat=curdat->_nextHeaderData){
        hsize+=curdat->_Keylen+curdat->_Valuelen;
      }
      return hsize;
    }
    

Anmelden zum Antworten