Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / qt / 2007-07

Some moments with commit 11541 (Fxed ugly memleak with TQtImage)

2 messages · 2 people ·

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Павел Ишенин

Hello, Items specific to the Qt widget set

Can I advance a remark about commit?

If so, then I will do :)

To be closer with other lazarus sources and internal rules I want to comment some aspects of patch.

First of all - naming:

class fields usually starts with letter F while arguments with letter A, so ACreateData: PByte; ADispose: Boolean; better to write as FData: PByte; FDataOwner: Boolean;

also CreateData tell us that this is someting data that either we want to create or ... I dont know :) FData is more general. ADispose tell us nothing about what to Dispose while FDataOwner tell us then TQtImage is Owner of FData.

Second problem - destructor.

If we allocated memory with GetMem / AllocMem then we must free it with FreeMem while if we allocated memory with New then we must use Dispose.

So, I changed Dispose to FreeMem.

I attached patch. I have not checked it compilation I only edited source file in text editor. So, please test it and better apply.

I cant force you to use suggested fields names (better to use - it will be easier to read code later), but FreeMem should be use instead of Dispose.

And at last - sorry for grumbling :)

Best regards,
Paul Ishenin.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed... Name: qt_image.patch
URL: <http://lists.lazarus.freepascal.org/pipermail/qt/attachments/20070717/c372d318/attachment.ksh>

zeljko
to Павел Ишенин
????? ?????? wrote 5 lines
Hello, Items specific to the Qt widget set Can I advance a remark about commit? If so, then I will do :)

yep ;)

Show quoted text 14 lines
To be closer with other lazarus sources and internal rules I want to comment some aspects of patch. First of all - naming: class fields usually starts with letter F while arguments with letter A, so ACreateData: PByte; ADispose: Boolean; better to write as FData: PByte; FDataOwner: Boolean; also CreateData tell us that this is someting data that either we want to create or ... I dont know :) FData is more general. ADispose tell us nothing about what to Dispose while FDataOwner tell us then TQtImage is Owner of FData.

ok

Show quoted text 15 lines
Second problem - destructor. If we allocated memory with GetMem / AllocMem then we must free it with FreeMem while if we allocated memory with New then we must use Dispose. So, I changed Dispose to FreeMem. I attached patch. I have not checked it compilation I only edited source file in text editor. So, please test it and better apply. I cant force you to use suggested fields names (better to use - it will be easier to read code later), but FreeMem should be use instead of Dispose. And at last - sorry for grumbling :)

I've tried with FreeMem(), but it won't compile (probably my typo), so I change it to Dispose() just to test, and then forgot to fix it ;)

tnx for the patch :)