Amatő C-program építő jellegű kritizálása

The code is quite extensive, and a thorough review would require a deep understanding of the overall architecture and purpose of the application. However, I can point out some potential issues and areas for improvement based on general programming practices and SDL usage. Here are a few observations:

1. Use of system Calls: The use of system calls for executing shell commands (like ls) is generally discouraged because it can introduce security vulnerabilities and is less portable. It's better to use direct filesystem API calls provided by the standard library or the OS.

2. Error Handling: There are several places where the return value of functions (like SDL_CreateWindow, SDL_CreateRenderer, TTF_RenderUTF8_Blended, MagickReadImage, etc.) is not checked for errors. Proper error handling is crucial to make the program robust and easier to debug.

3. Resource Management: There are potential resource leaks. For example, SDL_DestroyTexture is not consistently called for every texture created with SDL_CreateTextureFromSurface. Make sure that all allocated resources are properly released.

4. Global Variables: The use of global variables (like Loading_Surf, Zene_Log_Message, etc.) is generally discouraged as it can lead to code that is hard to maintain and debug. It's better to pass these as parameters or use them in a more localized scope.

5. Hardcoded File Paths: File paths are hardcoded in the program, which reduces portability and flexibility. Consider allowing the user to specify paths through configuration files or command line arguments.

6. Magic Numbers: There are several magic numbers in the code. Replace them with named constants or configurable parameters to make the code more readable and maintainable.

7. Thread Management: There is a potential issue with thread management where pthread_cancel is used. This might lead to resources not being properly released. Consider using a more controlled way of signaling threads to exit.

8. Lack of Comments: The code lacks comments explaining the purpose of functions and blocks of code, which makes it hard for someone else to understand what each part of the code is supposed to do.

9. Code Formatting: The code formatting is inconsistent in places (e.g., indentation, spacing). Consistent formatting makes the code more readable.

10. Potential Buffer Overflows: There are several instances where strings are copied into fixed-size buffers without proper bounds checking, which could lead to buffer overflows.

Tessék, nulladik körnek egy review, validnak tűnnek a dolgok benne.

Áttekinthetőség szempontjából végülis nem rossz, de ami függvényeket kívülről nem hívsz, azokat nem feltétlenül kell a header fájlban deklarálni, elég a .c-ben és static-ba is lehet rakni.

Ja meg a header fájlokban az endif az a fájl végére menjen. :)

Ahhoz képest, hogy nem vagy programozó, egész jó. Néhol van benne behúzási inkonzisztencia, de égbe kiáltó hülyeséget így hirtelen átfutva nem találok benne. Az is igaz, hogy nem mélyedtem bele elég alaposan a kódba, hogy mit csinál. El sem hinnéd, hogy hivatásos programozók néha milyen förmedvényt, meg karban tarthatatlan gányolásokat adnak ki a kezük közül, a te kódod még azokhoz képest elég szabályos, vállalható, könnyen érthető. Sok projekt kódját épp ezért nem nyitják meg, nem a nyerészkedés az oka, hanem mert nem merik benne a gányolást, meg a mástól ellopott kódokat felvállalni, rendesre megírni meg sajnálják a munkát beletenni ahhoz, hogy másnak ingyen hasznosuljon a kód. A YouTube-on van is néhány csatorna, ami ilyen projektek kiszivárgott forráskódjából (GTA V, Simpsons Hit & Run, stb.) idézget fejlesztői kommenteket, amikből kiderül, hogy egy csomó kódrész csak egy csúnya hack, meg maguk se értik, hogy mit csinál, miért kell.

Persze, apróságokba mindig bele lehet kötni, pl. hogy void függvény végére minek return(0); ha úgy sincs visszatérési érték, de ez végül is nem abszolút értelemben hiba, sőt, egyesek paranoiaként kifejezetten beleteszik, akkor is, ha felesleges, hogy ha valami régi, nem sztenderd fordítóval fordítják, az ne ugasson, hogy nincs a függvényben egy return se.

Amibe talán kicsit belekötnék, hogy ha fejlesztesz, akkor git tárolóba told ki a cuccot, az ilyen mega.nz, sourceforge, Google Drive, OneDrive, pastebin, stb. a normik területe. A Code::Blocks teljesen jó, legalább minimalista, bár ha több funkciót akarsz annál, akkor egy VSCode azért hozhat 1-2 könnyítést kódírás közben.

Hát... ööö... ez valami zenegép lesz :D

Csak a main.c-be néztem bele, de én is tényleg csak apróságokat látok (vagy nem látok benne)

én így szoktam írni a függvényeket, nekem így átláthatóbb

void fuggveny(int Valami) {
   Ide jon = a(kod);
   if (sohanapja == kiskedd) {
      fuggveny(0);
   }
}

ez mondjuk megszokás kérdése, de ezt:

if((Text_Box.y+Text_Box.h)>(Playlist_Box.y+Playlist_Box.h)) { Playlist->Max_Win_Entries=n-1; break; }

én a helyedben így írnám:

if ( (Text_Box.y+Text_Box.h) > (Playlist_Box.y+Playlist_Box.h) ) 
{ 
    Playlist->Max_Win_Entries = n-1; 
    break; 
}

és akkor egységesebb lenne az egész.

Egyébként vanna erre mindenféle csilli-villi kód formázók, amik előre beállított szabályok alapján kicicomázzák a kódodat.

Aztán nem sok kommentet látok benne. Vagyis ami van, az csak biztonsági okokból, (később még jó lesz), benne hagyott, egy-két kódrészlet. Én azokat a kódokat is amit tuti, hogy rajtam kívül nem fog más olvasni, bőven teletűzdelem kommentekkel. Sőt, sokszor úgy kezdem egy függvényt írni, hogy először kommentben szépen leírom az algoritmust, kvázi pszeudokód jelleggel, és utána már nem kell a doskit nézegetni, elég csak a kommentet megvalósítani. Nálam nem ritka, hogy több a komment mint a kód.

Ja és ha már komment, akkor a fájlok fejlécében meg szokás emlékezni arról, hogy ki, mikor, miért csinálta, és arról is, hogy ki és milyen feltételekkel használhatja.

A warez részhez nem szólok semmit :D

Hajrá!

Nem csak ehhez, hanem úgy általánosságban is, szerintem a legjobb, ha az ellenőrzést a fordítóra bízod:

-Wall -Wextra mindenképp, rengeteg hibát kiszúr, még a memóriakezelési hibákat is, már amit statikus analízissel ki lehet szúrni.

-pedantic ha szeretnéd, hogy azt is ellenőrizze, hogy szépen van-e megírva a forrásod.

Ezen kívül minden olyanra, ami statikus analízissel nem észlelhető, mindenképp javaslom legalább egyszer -g kapcsolóval fordítani, és valgrind-en keresztül is lefuttatni. Tetves lassan fog futni, de meglepően pontos riportot ad. Pl ellenőrzi a futásidejű buffer overflow-okat, out of bound eléréseket, inicializálatlan memória használatot, és ha elfelejtetted meghívni a free()-t, akkor azt is megmondja, melyik fájl hányadik sorában volt hozzá a malloc().

A valgrind --leak-check=full --show-leak-kinds=all --num-callers=32 kapcsolók hatására méglassabb lesz, de mégpontosabb hibajelzéseket ad, pl. ha meghívtad egy függvénykönyvtár inicializálóját, ami memóriát foglalt, akkor jelezni fogja, ha elfelejtetted meghívni a függvénykönyvtár destruktorát. Ennek csak az az egy hátulütője van, hogy az SDL által linkelt libdl és libasound ezerrel memleakel, és a zajtól semmit sem fogsz látni. Erre megoldás a --suppressions=libSDL.supp kapcsoló.

Ezen kívül a modern fordítók is képesek futásidejű ellenőrzéseket belefordítani a kódba a -fsanitize=undefined kapcsoló hatására, lásd ASAN, UBSAN (pl. CLang, gcc), bár én ezeket nem szeretem túlságosan, mert nem túl megbízhatóak. Rengeteg munkaórát öltem már abba, hogy lenyomozzak egy-egy riasztást, csakhogy kiderüljön, nem is a kódomban volt a hiba, hanem fals pozitív volt a riportjuk. De irányadónak mindenképp. UB detektálásra jó trükk, ha -O3 kapcsolóval is lefordítod, az optimalizáló ezen a szinten már ugyanis besír rájuk (ilyenkor nem mindig fogsz pontos hibaüzenetet kapni, de tapasztalatom szerint megbízhatóbb a detektálásban, mint az UBSAN).

Mindenesetre én a helyedben mindenképp először a fordítómra bíznám az ellenőrzést, aztán a valgrind-ra, és nem pedig egy olyan ChatGPT-re, ami folyton hallucinál és a fene tudja, mennyire megbízható, amit épp kiköpött magából.

linter-t kérdezted már?

A kommentek nagyon jók, csak nem maradnak szinkronban a kóddal, tele lesz a program ilyenekkel:

/* a nyugdíjkorhatár 62 év */
if (ev-szulEv >= 65) ...

alternatíva:

int NyugdijKorhatar=65;
int kor=ev-szulEv;
if (kor >= NyugdijKorhatar) ...

Off: persze majd egy következő menetben azon is elgondolkozunk, hogy ez eletkort nem így számoljuk. (De még ez is jobb, mint az a naív ötlet, hogy az emberek korát tároljuk, és azt évente aktualizáljuk.)

Feltöltöttél publikusba feltételezhetően szerző jog által védett zenét (nem hallgattam bele, csak a fájlnevek alapján tippelem) és belinkelted nekünk? Nem tűnik túlzottan bölcs húzásnak.

Nyomdahibák tömkelege (itt a postod címének első szavában is), "tittle", "equlizer", "entri", "albumborrító", "méj", nem is folytatom a vadászatot.

Angol és magyar indokolatlan keverése a változónevekben és a log üzenetekben is.

A log üzenet buffere 200 byte-os, az üzenet meg simán tartalmaz előadót, számcímet, simán túlcsordulhat csúnya memóriakorrupcióhoz vezetve.

Következetlen formázás. Az indentálás hol 2, hol 4, néha egy metóduson belül szintenként változva. main.c:497-nél záró kapocs nélkül kijjebb megy az indentálás, az ilyet a szemednek azonnal ki kéne szúrnia hogy nincs rendben (manapság a gcc is kiszúrja). 612. sor ugyanez. 662. sor ha a tab nálad 4-et jelent akkor jól néz ki, de nálam 8 és így rossz.

Következetlen, hogy teszel-e szóközt "if" után, struktúranév elé, vessző elé (paraméterek közé), teszel-e zárójelet a return után, stb. Egy egységes, lehetőleg sok projektben látott, "standard" formázás sokat segít hosszútávon ha nagyobb méretű projektekbe vágsz bele.

Kissé konkrétabb, és egyben szubjektívabb észrevételek a formázással kapcsolatban:

A zárójelezős "return(0)"-tól kiver a frász, tapasztalatom szerint általában azok írják ezt, akik úgy képzelik, hogy ilyenkor meghívjuk a "return" nevű függvényt egy nullás paraméterrel.

Bevett gyakorlat, hogy beépített kulcsszó ("if", "for", "while", "switch" stb.) után van szóköz a zárójel előtt, függvényhíváskor viszont nincs.

Általában egy picit (nem sokkal, csak picit) jobban széthúzás olvashatóbbá teszi a kódot. Például "if((Text_Box.y+Text_Box.h)>(Playlist_Box.y+Playlist_Box.h))"  helyett "if ((Text_Box.y + Text_Box.h) > (Playlist_Box.y + Playlist_Box.h))" mennyivel olvashatóbb első ránézésre, a szóközök segítik a szemet gyorsan parse-olni a kódot.

A zárójelekből is értelmes kompromisszumot kell keresni. A túl kevés sem mindig jó, de a minél több sem. Például ha két összeadás eredményét hasonlítjuk össze, tök fölösleges, szerintem csak lassítja az olvasást. Az előzőnél szerintem még egyszerűbben olvasható az hogy "if (Text_Box.y + Text_Box.h > Playlist_Box.y + Playlist_Box.h)", és egy pillanatig sem lehet kérdés, hogy hogyan is kell értelmezni.

Nagyjából a külalakra (és nem az összetettebb dolgokra, átfogó struktúrára) fókuszálva egyelőre ennyi.

Az EXIT_SUCCESS / EXIT_FAILURE értékeket a main()-ből visszatéréskor szokás használni, ahol az értékek "fordítva" vannak, mint C-ben kódoláskor megszokja az ember és értelme van. A zene_log()-ban ezért félrevezető ezt adni vissza. Ha valahol használni akarod a visszatérési értékét, akkor valszeg úgy akarod használni, hogy "if (zene_log(...)) { siker... } else { hiba... }", ehhez nem jó a mostani visszatérési érték.

Ahogy a parancssort megalkotod, idézőjelek közé téve a paramétert (előadót, számcímet), majd system()-mel odaadva az mplayer-nek, szét fog esni bizonyos speciális karaktereknél (idézőjel, dollár, backtick, backslash stb.). Vagy írj egy rendes shell számára escape-elő metódust, vagy system() helyett posix_spawn() vagy fork()+execve() és társai.

A többszálúsítás iszonyú aknamező, nagyon jól kell tudni programozni hogy jól csinálja az ember. Mindkét szálad ugyanabban a pufferben állítja össze a log üzenetet majd írja ki, ez kapásból rossz ha kábé egyszerre fut rá a két szál, simán lehet hogy az egyik üzeneted jelenik meg kétszer és a másik egyszer sem, vagy valami még nagyobb zagyvaság. Az SDL-hez nem értek; hacsaknem írja a doksija hogy kifejezetten szabad többszálúsítani, valószínűleg ott is össze tud akadni a két szál.

Megnéztem a szótárban, az efféle határozószavas alakokat külön kell írni: ama tő. :))