AuthDB

Quiche Lorraine

weil die Stacksize dort 128kb nur groß ist

Also der folgende Ausdruck sizeof(std::make_unique<char>()); liefert bei mir 4 zurück. Also ist das eher kein Kandidat welcher Stack frisst.

Und 128 kByte ist schon ok, sofern man keine Rekursion macht.

Tuxist1

ich sollte ich mich mehr mit den neuen Sprach Standarts beschäftigen habt ihr recht.

C nutze ich auch gerne das stimmt. Wo mit überhaupt nicht zurecht komme ist Java ist mir zu abstrakt zu Umständlich.

Ich denke das werde ich alles nochmal überarbeiten sobald ich die API am laufen habe.

Teile meiner Biobiotheken habe auch schon 10 Jahre oder mehr auf den Buckel.

Das ich verkette Listen benutze liegt daran das ich es nicht mag Allokatoren zu schreiben finde das total umständlich und mit liste schneller und einfacher vielleicht habe ich da auch unrecht.

wob

Du kannst auch mal clang-tidy verwenden. Wenn du cmake aufrufst, verwende
cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ...deine Argumente...
Und dann kannst du clang-tidy datei.cpp -p build-directory ... checken.

Es kommt z.B. raus:

authdb/src/session.cpp:165:26: warning: Potential leak of memory pointed to by 'grpdat.gid' [clang-analyzer-cplusplus.NewDeleteLeaks]
  165 |         _lastData->_next=new SessionData(sid,userid,domainid,grpdat.gid,grpdat.count);
      |                          ^
authdb/src/session.cpp:105:11: note: Assuming 'rd' is >= 'end'
  105 |     while(rd<end){
      |           ^~~~~~
authdb/src/session.cpp:105:5: note: Loop condition is false. Execution continues on line 134
  105 |     while(rd<end){
      |     ^
authdb/src/session.cpp:134:18: note: Memory is allocated
  134 |     grpdat.gid = new uuid_t[grpdat.count];
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~
authdb/src/session.cpp:138:11: note: 'rd' is >= 'end'
  138 |     while(rd<end){
      |           ^~
authdb/src/session.cpp:138:5: note: Loop condition is false. Execution continues on line 158
  138 |     while(rd<end){
      |     ^
authdb/src/session.cpp:161:8: note: Assuming field '_firstData' is non-null
  161 |     if(!_firstData){
      |        ^~~~~~~~~~~
authdb/src/session.cpp:161:5: note: Taking false branch
  161 |     if(!_firstData){
      |     ^
authdb/src/session.cpp:165:26: note: Potential leak of memory pointed to by 'grpdat.gid'
  165 |         _lastData->_next=new SessionData(sid,userid,domainid,grpdat.gid,grpdat.count);
      |                          ^
authdb/src/session.cpp:255:5: warning: Argument to 'delete[]' is uninitialized [clang-analyzer-core.CallAndMessage]
  255 |     delete[] grpdat.gid;
      |     ^~~~~~~~~~~~~~~~~~~
authdb/src/session.cpp:215:5: note: Loop condition is true.  Entering loop body
  215 |     for(SessionData *cur=_firstData; cur; cur=cur->_next){
      |     ^
authdb/src/session.cpp:216:12: note: Assuming the condition is true
  216 |         if(uuid_compare(cur->_sid,sessionid)==0){
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
authdb/src/session.cpp:216:9: note: Taking true branch
  216 |         if(uuid_compare(cur->_sid,sessionid)==0){
      |         ^
authdb/src/session.cpp:218:13: note:  Execution continues on line 222
  218 |             break;
      |             ^
authdb/src/session.cpp:222:9: note: 'cursess' is non-null
  222 |     if(!cursess)
      |         ^~~~~~~
authdb/src/session.cpp:222:5: note: Taking false branch
  222 |     if(!cursess)
      |     ^
authdb/src/session.cpp:226:11: note: Assuming 'rd' is >= 'end'
  226 |     while(rd<end){
      |           ^~~~~~
authdb/src/session.cpp:226:5: note: Loop condition is false. Execution continues on line 255
  226 |     while(rd<end){
      |     ^
authdb/src/session.cpp:255:5: note: Argument to 'delete[]' is uninitialized
  255 |     delete[] grpdat.gid;
      |     ^~~~~~~~~~~~~~~~~~~

Du kannst mit -check=\* auch andere Checks aktivieren (mit dem * sind es aber zu viele, da dann auch viele Style-Dinge dabei sind, die man oft nicht will).
Da fällt dann sowas auf:

authdb/src/session.cpp:144:22: warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
  144 |         backend.read((unsigned char*)&cur,sizeof(AuthData::Record));

oder sowas:

authdb/src/session.cpp:232:22: warning: C-style casts are discouraged; use reinterpret_cast [google-readability-casting]
  232 |         backend.read((unsigned char*)&cur,sizeof(AuthData::Record));
      |                      ^~~~~~~~~~~~~~~~    
      |                      reinterpret_cast<unsigned char*>( )

Quiche Lorraine

BTW:
So wie es aussieht frisst die Funktion authdb::sha512::transform aus dem Stand 1 kByte Stack (lokale Parameter (w, wv,...) + Klassenvariablen (m_block,...).

BTW: Kryptographische Funktionen sollte man nicht selbst programmieren, da zu trickreich und fehleranfällig. Hast du wenigstens eine Testreihe für sha512.cppaufgebaut?

Tuxist1

@Quiche-Lorraine soweit bin ich noch nicht gekommen ist ja erst eine Techpreview soll mit in die unit tests fließen mit welchen ich nocht nicht angefangen habe aber danke für den Hinweiß

Tuxist1

        struct Groups{
            size_t  count = 0;
            uuid_t *gid = nullptr;
        } _member;

gegen std::array tauschen ?

wob

@Tuxist1 sagte in AuthDB:

ist ja erst eine Techpreview soll mit in die unit tests fließen mit welchen ich nocht nicht angefangen habe

Sorry, aber so wird das nie funktionieren und dir auch nicht helfen. Schreib den Test gleich zusammen mit der Funktion! Später macht man es eh nicht. Bei sowas wie dem sha-Code könnte man die Tests auch problemlos vor der Funktion schreiben. (Manche sagen, dass man das immer machen soll, aber ich schreibe Tests auch mal gerne nach der Funktion, besonders wenn ich um die problematischen Stellen der Implementierung bescheid weiß und diese ganz gezielt testen will). Was noch niche funktioniert hat, ist die Tests erst viel später zu schreiben. Du willst doch auch von einer gut getesteten Codebase profitieren, z.B. bei einem Refactoring sicher(er) sein, dass noch alles geht.

Quiche Lorraine

@Tuxist1 sagte in AuthDB:

gegen std::array tauschen ?

Würde ich so empfehlen. Vor allen Dingen kannst du so auf einmal dein Struct kopieren, ohne dass du Seiteneffekte bekommst.

Ich würde aber auch zuerst empfehlen Testreihen zu schreiben, um dein Verhalten festzuzurren. Wie ein Korsett schnürst du dann dein Programm ein und siehst bei Änderungen direkt, ob sich dein Verhalten verändert hat.

Tuxist1

die session.cpp habe ich jetzt stark modifiziert und an den sha code gehe morgen.

wob

Wenn ich mir die 17 Commit alle mit dem Titel "fixed session.cpp" angucke, frage ich mich, ob das wirkliche Fixes sind oder teilweise eher style-Improvements.

z.B. im letzten Commit:

-            std::shared_ptr<unsigned char[]>tmp(new unsigned char[cur.datasize]);
+            const std::shared_ptr<unsigned char[]>tmp(new unsigned char[cur.datasize]);
             backend.read(tmp.get(),cur.datasize);
             username=reinterpret_cast<char*>(tmp.get());
         }

Statt const vs non-const stellen sich mir hier ganz andere Fragen:

Warum überhaupt ein shared pointer?! Der wird nicht gesharet! Sollte unique_ptr sein!
Shared- und unique-Pointer werden mit std::make_shared und std::make_unique angelegt, nicht manuell mit new! Siehe R22 und R23 in https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-make_shared
Ist überhaupt sichergestellt, dass backend.read etwas nullterminiertes zurückgibt?
Könnte man nicht dem Backend direkt beibringen, in einen string zu lesen, sodass man die tmp gar nicht braucht?