[C++] geheugen vrijgeven

[Verwijderd]

Ik heb een programma geschreven dat handmatig bmp en pcx bestanden inleest. Dit werkt dus met een klasse bmpformat (of pcx dus), die bestanden lezen en weergeven.

Als ik dus zo'n klasse heb, moet ik dan niet, nadat mijn bitmap gedisplayed is, het geheugen weer vrijgeven? Er staat namelijk op dat moment een heel figuur bestand in, maar ik weet niet echt hoe dit moet.. De code:

C++:

bmpformat bmp(filename);
bmp.dispbitmap(GetDC(hwnd)); //wordt dus zo gedeclareerd enzo, moet hier dus weer iets onder van free bmp ofzo.

en het bmp bestand:

C++:

class bmpformat{


public :

    
  bmpformat(char * filename) {//bmp inlees (constructor van klasse bmpformat)
  FILE * file1 = fopen(filename,"rb");
  fread(&bmp.header.id,1,sizeof(bmp.header.id),file1);  
  if (bmp.header.id != 19778) { fclose(file1); exit(0); }//Als het geen windows bitmap bestand sluiten.
  fread(&bmp.header.filesize,1,sizeof(bmp.header.filesize),file1);  
  fread(&bmp.header.reserved,1,sizeof(bmp.header.reserved),file1);  
  fread(&bmp.header.dataoffset,1,sizeof(bmp.header.dataoffset),file1);  
  fread(&bmp.header.headersize,1,sizeof(bmp.header.headersize),file1);  
  fread(&bmp.header.width,1,sizeof(bmp.header.width),file1); 
  fread(&bmp.header.height,1,sizeof(bmp.header.height),file1);  
  fread(&bmp.header.planes,1,sizeof(bmp.header.planes),file1);  
  fread(&bmp.header.bpp,1,sizeof(bmp.header.bpp),file1);  
  fread(&bmp.header.compression,1,sizeof(bmp.header.compression),file1);
  if (bmp.header.compression != 0){fclose(file1); exit(0);}
  fread(&bmp.header.bitmapdatasize,1,sizeof(bmp.header.bitmapdatasize),file1);  
  fread(&bmp.header.hresolution,1,sizeof(bmp.header.hresolution),file1);  
  fread(&bmp.header.vresolution,1,sizeof(bmp.header.vresolution),file1);  
  fread(&bmp.header.colors,1,sizeof(bmp.header.colors),file1);       
  fread(&bmp.header.importantcolors,1,sizeof(bmp.header.importantcolors),file1);
  switch (bmp.header.bpp){
    case 1:
    case 4:
    case 8:{
          int palsize = (int)(pow(2,bmp.header.bpp)*4);
          bmp.palette = (bmpformat::kleurstruct*) malloc (palsize);
          fread(bmp.palette,1,palsize,file1);
          int datasize =(int)(bmp.header.filesize-bmp.header.dataoffset);//photoshop doesn't store bitmapdatasize, so calculate it manually.
          bmp.bitmapdata  = (char*) malloc (datasize);
          fread(bmp.bitmapdata,1,datasize,file1);
          break;
          }
    case 24:{//24 bits heeft geen palette
          int datasize =(int)(bmp.header.filesize-bmp.header.dataoffset);
          bmp.bitmapdata  = (char*) malloc (datasize);
          fread(bmp.bitmapdata,1,datasize,file1);
          break;
        }
    default:
        break;
    }
    fclose(file1);
}


private:
    struct bmpheader {//De header van de bitmap.
        short id;
        unsigned int filesize;
        int reserved;
        unsigned int dataoffset;//begin van de data
        unsigned int headersize;
        unsigned int width;
        unsigned int height;
        unsigned short planes;
        unsigned short bpp;//bits per pixel
        unsigned int compression;
        unsigned int bitmapdatasize;
        unsigned int hresolution;
        unsigned int vresolution;
        unsigned int colors; 
        unsigned int importantcolors;
    };

    struct kleurstruct//in een bitmap zijn de kleuren in omgekeerde volgorde opgeslagen.
    {                         
     char b;
     char g;
     char r;
     char unused;//ter opvulling, afronding op 4.
    };

    struct bitmap
    {
     bmpheader header; 
     kleurstruct * palette;
     char * bitmapdata;
    }; //definieer type bitmap
    
    bitmap bmp;
    
    public:


void dispbitmap(HDC hdc) {
  switch (bmp.header.bpp) {//check bits per pixel
    case 1:{//1bpp
         unsigned int trunc;
         for (int i=0;i<32;i++) {
             if ((int)(bmp.header.width+i)/32 == (float)(bmp.header.width+i)/32) { trunc = i;}//delen door 32 ipv 4 omdat het hier om bits en niet om bytes
         }
         for (unsigned int y=0;y<bmp.header.height;y++) {
            for (unsigned int x=0;x<bmp.header.width;x++) {
                int datapos = y*(bmp.header.width+trunc)+x;
                int bytetoread = ((datapos)/8);
                int bittoread = datapos-(8*((unsigned int)(datapos/8)));
                int dataread = (((unsigned char)((unsigned char)bmp.bitmapdata[bytetoread]*(pow(2,bittoread))))/128);
                SetPixel(hdc,x,bmp.header.height-y,RGB(
                    bmp.palette[dataread].r,
                    bmp.palette[dataread].g,
                    bmp.palette[dataread].b)); 
            }
         }
         break;
         }
    case 4:{//4 bits
         unsigned int trunc;
         for (int i=0;i<4;i++) {
            if ((int)(bmp.header.width+i)/4 == (float)(bmp.header.width+i)/4) { trunc = i; }//de breedte wordt op 4 afgerond.
         }
         for (unsigned int y=0;y<bmp.header.height;y++) {
            for (unsigned int x=0;x<bmp.header.width;x++) {
                int datapos = (y*trunc)+(y*bmp.header.width+x);
                int dataread;
                if ((int)(datapos/2) == (float)datapos/2) { dataread = ((unsigned char)bmp.bitmapdata[datapos/2])/16; }
                else { dataread = ((unsigned char)bmp.bitmapdata[datapos/2])-(16*(((unsigned char)bmp.bitmapdata[datapos/2])/16)); } 
                SetPixel(hdc,x,bmp.header.height-(y+1),RGB(
                    bmp.palette[dataread].r,
                    bmp.palette[dataread].g,
                    bmp.palette[dataread].b)); 
            }
         }
         break;
         }
    case 8:{//8 bits
         unsigned int trunc;
         for (int i=0;i<4;i++) {
            if ((int)(bmp.header.width+i)/4 == (float)(bmp.header.width+i)/4) { trunc = i; }
         }
         for (unsigned int y=0;y<bmp.header.height;y++) {
            for (unsigned int x=0;x<bmp.header.width;x++) {
                int datapos = y*(bmp.header.width+trunc)+x;
                int dataread = (unsigned char)bmp.bitmapdata[datapos];
                SetPixel(hdc,x,bmp.header.height-(y+1),RGB(//er zat nog een witte rand, dus ff 1 regel meer eraf halen
                    bmp.palette[dataread].r,
                    bmp.palette[dataread].g,
                    bmp.palette[dataread].b)); 
            }
         }
        break;
        }
    case 24:{//24 bits
         unsigned int trunc;
         int datapos=0;
         int datasize =(int)(bmp.header.filesize-bmp.header.dataoffset);
         for (int i=0;i<4;i++) {
            if ((int)(bmp.header.width+i)/4 == (float)(bmp.header.width+i)/4) { trunc = i; }
         }
         for (unsigned int y=0;y<bmp.header.height;y++) {
            for (unsigned int x=0;x<bmp.header.width;x++) {
                if (datapos < datasize) {datapos = (y*trunc)+(3*(y*bmp.header.width+x));}
                SetPixel(hdc,x,bmp.header.height-(y+1),RGB(
                        bmp.bitmapdata[datapos+2],//(in bestand staat het (b,g,r) dus om r te krijgen moet je 2 verder springen, enz
                        bmp.bitmapdata[datapos+1],
                        bmp.bitmapdata[datapos])); 
            }
        }
        break;
        }
    }
}



};

[whoami]

Allereerst:
beperk je code eens tot de relevante code. Aan zo'n hap code hebben we helemaal niks, en het nodigt ook niet uit tot lezen.

Dynamisch gealloceerd geheugen kan je vrijgeven dmv de delete operator. Dat staat ook wel in ieder boek/tutorial waarin pointers behandeld worden.

[drm]

Even tussendoor: Ik zie heel veel dergelijke regels staan:

C++:

fread(&bmp.header.filesize,1,sizeof(bmp.header.filesize),file1);

Waarom lees je niet gewoon de hele header in een keer in met

C++:

fread ( &bmp.header, 1, sizeof(bmp.header), file1 );

Als je in de header (is een struct neem ik aan) gewoon de hele reutemeteut op de juiste volgorde met de juiste typen er inzet, kan je de hele header in 1 keer inlezen. Scheelt een hoop tikwerk en overzichtelijkheid.

[.oisyn]

drm schreef op 05 June 2003 @ 11:10:
Even tussendoor: Ik zie heel veel dergelijke regels staan:

C++:

fread(&bmp.header.filesize,1,sizeof(bmp.header.filesize),file1);

Waarom lees je niet gewoon de hele header in een keer in met

C++:

fread ( &bmp.header, 1, sizeof(bmp.header), file1 );

Als je in de header (is een struct neem ik aan) gewoon de hele reutemeteut op de juiste volgorde met de juiste typen er inzet, kan je de hele header in 1 keer inlezen. Scheelt een hoop tikwerk en overzichtelijkheid.

omdat dat door alignment niet hetzelfde kan zijn als de physieke file-layout
(Een struct van een short gevolgd door een int is meestal 8 bytes, niet 6, omdat die int op 4 bytes aligned wordt. En met files wil je dat meestal nou juist niet)


FD-man: alloceer je ergens in je class geheugen? Nee. Moet je het dus vrijgeven? Nee. Simpel eh?

[ Voor 7% gewijzigd door .oisyn op 05-06-2003 11:19 ]

[drm]

.oisyn:
omdat dat door alignment niet hetzelfde kan zijn als de physieke file-layout
(Een struct van een short gevolgd door een int is meestal 8 bytes, niet 6, omdat die int op 4 bytes aligned wordt. En met files wil je dat meestal nou juist niet)

Maar je kunt toch dmv. bitfields opgeven hoe groot je (fysiek) elke member wilt hebben?

edit:

Of wordt dat gewoon altijd aligned?

[ Voor 7% gewijzigd door drm op 05-06-2003 11:18 ]

[.oisyn]

Als je het met bitfields aangeeft dan wordt die field zoveel bits breed. Dat zegt nog niets over de positie en de alignment in de struct. De compiler kan er zelfs voor kiezen om members door elkaar te husselen voor een optimaler resultaat. Bovendien werken bitfields alleen op integral types

De meeste compilers komen wel met alignment directives, maar dat is weer niet standaard. Member-voor-member in- en uitlezen is dus de enige correcte C++ manier

[.oisyn]

C++:

if (bmp.header.id != 19778) { fclose(file1); exit(0); }

Dus als iemand jouw class gebruikt en een ander bestand inlaadt dan een bmp dan wordt het programma afgesloten? Help me te herinneren dat ik nooit code van jou in gebruik ga nemen

[drm]

.oisyn:
Als je het met bitfields aangeeft dan wordt die field zoveel bits breed. Dat zegt nog niets over de positie en de alignment in de struct. De compiler kan er zelfs voor kiezen om members door elkaar te husselen voor een optimaler resultaat.

Dat had ik kunnen bedenken, idd...

De meeste compilers komen wel met alignment directives, maar dat is weer niet standaard. Member-voor-member in- en uitlezen is dus de enige correcte C++ manier

ok

_{kunnen we weer ontopic}

[Verwijderd]

.oisyn schreef op 05 juni 2003 @ 11:22:

C++:

if (bmp.header.id != 19778) { fclose(file1); exit(0); }

Dus als iemand jouw class gebruikt en een ander bestand inlaadt dan een bmp dan wordt het programma afgesloten? Help me te herinneren dat ik nooit code van jou in gebruik ga nemen

Hehe, lol .

Eigenlijk is deze klasse niet voor anderen he, alleen voor eigen proggie . Maar goed, voor netjes object-georienteerd programmeren kan het natuurlijk niet, ik zal er nog maar is effies wat aan veranderen .

sorry van de lap code idd, maar ik moest weg en had dus geen tijd meer om al het onnuttige er ff uit te halen .

Maar met dat geheugen hoeft dus niet, dat is mooi. Maar alle variabelen enzo eten toch geheugen (of wordt dat dan automatisch weggegooid?), laat staan de hele bitmapdata, 24-bits bitmaps willen nog wel is groot zijn .

Maar dan hoef ik daar dus niet meer op te letten, mooi makkelijk .

[whoami]

Geheugen van variabelen die op de stack gecreeërd zijn, wordt automatisch vrijgegeven als ze out - of - scope gaan.
Geheugen van variabelen die op de heap gecreeërd zijn, moet je zelf vrijgeven. (delete).

Ik zie dat je bestand op de heap gecreeërd is, dus dat zal je zelf moeten vrijgeven geloof ik.

[ Voor 20% gewijzigd door whoami op 05-06-2003 15:51 ]

[The End]

Wat je wel moet doen is dit:

code:

HDC DC = GetDC(hwnd);
bmp.dispbitmap(DC);
ReleaseDC(DC);

Anders krijg je resource leaks en kan je na een tijdje helemaal niks meer tekenen.

[.oisyn]

whoami schreef op 05 juni 2003 @ 15:51:
Ik zie dat je bestand op de heap gecreeërd is, dus dat zal je zelf moeten vrijgeven geloof ik.

Het is jammer dat die ene /14 modforce draad in HH staat, anders had ik je even verwezen naar mijn opmerking over het posten van jou in draadjes over dingen waar je geen verstand van had

fopen () alloceert idd een object op de heap, maar fclose () geeft dat geheugen weer vrij. Verder zijn alle variabelen lokaal. Aangezien de class zelf geen geheugen alloceert is het ook niet zijn verantwoordelijkheid om iets vrij te geven.

Degene die de class instantieert moet dat geheugen natuurlijk wel vrijgeven, maar dat gebeurt automatisch als het op de stack staat (wat in het programma dus het geval is)

[Verwijderd]

dus ditte is niet nodig? :

C++:

  ~bmpformat() {
  MessageBox(NULL, "Memory freed!", "bla",
            MB_ICONEXCLAMATION | MB_OK);
  delete[] bmp.bitmapdata,bmp.palette;
 
  }

En als ik nou is wel zelf het geheugen van het hele object in een keer wil vrijgeven? Ach laat maar, het is toch niet nodig .

[Verwijderd]

The End schreef op 05 June 2003 @ 16:06:
Wat je wel moet doen is dit:

code:

HDC DC = GetDC(hwnd);
bmp.dispbitmap(DC);
ReleaseDC(DC);

Anders krijg je resource leaks en kan je na een tijdje helemaal niks meer tekenen.

bmploader.cpp(59) : error C2660: 'ReleaseDC' : function does not take 1 parameters

[.oisyn]

Verwijderd schreef op 05 juni 2003 @ 16:20:
dus ditte is niet nodig? :

C++:

  ~bmpformat() {
  MessageBox(NULL, "Memory freed!", "bla",
            MB_ICONEXCLAMATION | MB_OK);
  delete[] bmp.bitmapdata,bmp.palette;
 
  }

En als ik nou is wel zelf het geheugen van het hele object in een keer wil vrijgeven? Ach laat maar, het is toch niet nodig .

hooooo wacht, ik heb die mallocs over het hoofd gezien.
Dus: je alloceert geheugen in je class, dus je moet dat zelf ook vrijgeven ja
En nee, niet met delete[], aangezien je het met malloc () hebt gealloceerd. Gebruik free ()

new T -> delete
new T[] -> delete[]
malloc () -> free ()

[.oisyn]

Verwijderd schreef op 05 juni 2003 @ 16:22:
[...]

bmploader.cpp(59) : error C2660: 'ReleaseDC' : function does not take 1 parameters

even in de MSDN kijken kan geen kwaad, denk je ook niet?

[curry684]

Eigenlijk is deze klasse niet voor anderen he, alleen voor eigen proggie . Maar goed, voor netjes object-georienteerd programmeren kan het natuurlijk niet, ik zal er nog maar is effies wat aan veranderen.

En voordat je die exit-call rechtstreeks in een return-statement veranderd even aan denken dat het object dan succesvol gecreeerd is, oftewel je beter een exception kunt gooien. En welke van de 2 je ook doet, even onthouden dat je dan wel alsnog die fclose aanroept aub

(ps. hint: exceptions kun je vangen en doorgooien, maar charmanter hier is om de file-access te encapsuleren zodat de destructor je vieze karweitjes kan doen)

[Eelis]

.

[ Voor 99% gewijzigd door Eelis op 18-02-2015 19:40 ]

[curry684]

En wat ik straks nog was vergeten: drm heeft wel degelijk gelijk met dat je alles beter in 1 keer in kunt lezen, maar dan moet je natuurlijk niet proberen een stapel standaardstructuren zelf te definieren: als je nu eens ipv bmpheader, kleurstruct en bitmapheader de door Windows zelf gedefinieerde varianten BITMAPINFOHEADER, RGBQUAD en BITMAPINFO gebruikt schiet het beter op Tevens is een BITMAPFILEHEADER die het begin van een bmp-bestand omvangt.

MSDN is your friend in dit geval, met de structuur van een bmp-file exact beschreven, en een voorbeeld met code van hoe je bmp-files zelf wegschrijft.

_{Stelletje prutz0rs}

[whoami]

.oisyn schreef op 05 June 2003 @ 16:09:
[...]


Het is jammer dat die ene /14 modforce draad in HH staat, anders had ik je even verwezen naar mijn opmerking over het posten van jou in draadjes over dingen waar je geen verstand van had

1. Dat topic staat niet in HH (want daar kunnen noch jij, noch ik in)
2. Ik had een disclaimer in m'n post gezet (zie de 'geloof ik')
3. STFU

[drm]

whoami:
1. Dat topic staat niet in HH (want daar kunnen noch jij, noch ik in)

Daarom kwam .oisyn ook aan mij vragen waar die draad gebleven was

3. STFU

vrede, heren, en ontopic aub

[Olaf van der Spek]

Eelis schreef op 05 June 2003 @ 18:35:
[...]

En voordat je nou iedere C api die je tegenkomt netjes gaat proberen te encapsuleren (wat enorm veel tijd kost), kun je in veel situaties beter gebruik maken van een scopeguard-variant die een functor naar keuze bij z'n destruction aanroept. Je krijgt dan zoiets:

C++:

FILE * f = fopen(...);
if (!f) throw ...;
ScopeGuard closer (boost::bind(&fclose, f));

waarna je naar het closen van f geen omkijken meer hebt. Op zich een redelijk comfortabele manier om om te gaan met interfaces die niet gebruik maken van nette C++ idiomen voor resource management, alleen jammer dat het composen van functors bij ietwat gecompliceerdere vrijgeef-code snel een enorme rommel wordt. Verder zijn scopeguards en functors helaas ook niet echt voor beginners weggelegd, met alle gevolgen van dien..

Encapsulatie is stukken netter en niet zoveel werk. Zeker als je ook reference-counting toevoegd.

[ Voor 5% gewijzigd door Olaf van der Spek op 05-06-2003 21:13 ]

[curry684]

Om maar een shortcut te noemen (erg kort door de bocht maar werkt wel ):

C++:

class File
{
private:
  FILE* m_Handle;

public:
  File(const char* p_Filename, const char* p_OpenMode)
    { m_Handle = fopen(p_Filename, p_OpenMode); }
  ~File()
    { if(m_Handle) fclose(m_Handle); }
  inline operator FILE*()
    { return m_Handle; }
};

[Olaf van der Spek]

Tot je per ongeluk de copy constructor of assignment operator gebruikt. Ik zou die of als private stub implementeren of volledig als public.

[Eelis]

.

[ Voor 99% gewijzigd door Eelis op 18-02-2015 19:40 ]

[curry684]

Olafvdspek: ik zei dus erg kort door de bocht, oftewel ik had geen tijd om meer te typen dan de miniem werkende constructie

[Olaf van der Spek]

Eelis schreef op 05 June 2003 @ 22:31:
De directe relatie met reference counting ontgaat me overigens.

C++:

File b;
{
    File a("c:/temp/t.txt", "w")
    b = a;
}
fwrite(b, ...)

Als je hier de code van Curry gebruikt, is de handle bij fwrite al gesloten. Dan moet je of de handle dupliceren (OS reference count verhogen) of zelf reference counting toepassen.

curry684 schreef op 05 June 2003 @ 22:31:
Olafvdspek: ik zei dus erg kort door de bocht, oftewel ik had geen tijd om meer te typen dan de miniem werkende constructie

Geef dan in ieder geval aan dat deze bug erin zit.