Chyba dne

vložil Radek Červinka 27. srpna 2016 00:23

Při jedné konzultaci jsem našel zajímavou chybu v jedné části zákazníkova programu. Myslím, že je to pěkný příklad.

//LEAK !
var
  ads: TADOQuery;
begin
  ads := TADOQuery.Create(nil);
  ads.Connection := ADOConnection1;
  with ads, SQL do
  begin
    try
      Text := 'SELECT * FROM tCRM_Currency';
      Open;
      try
        while not Eof do
        begin
          //………..
          Next;
        end;
      finally
        Close;
      end;
    finally
      Free;
    end;
  end;
end;

Zkuste ji první najít samostatně.

Myslím si, že použití with je pěšina do pekla. Vícenásobné with (nebo vnořené) je pak už dálnicí.

Pokud to není jasné, tak Free je voláno nad SQL, což je TStringList. Mimochodem chyba mohla vzniknout i např. při migraci z jiných komponent, které nemají SQL jako třídu a autor si chtěl ušetřit práci, tj. nemusela to být přímo chyba při originálním vývoji.

Každopádně každé použití with dobře zvažte, nikdy nevíte kdy se něco přidá do with-nuté části a Váš kód bude dělat něco jiného. Lokální proměnná je dost dobré řešení a lepší se s ní pak i pracuje při ladění (dá se např. zobrazit v debuggeru).


Nabízíme Delphi školení na různá témata, primárně ve Vaší firmě.

Tagy:

Praxe

Komentáře

27.8.2016 7:03:58 #

pepak

Podotknul bych, že tu potenciálně dochází k leaku i při odstranění with.

pepak

27.8.2016 9:53:03 #

pf1957

Musim se priznat, ze asi jako kazdy pascalista jsem with pouzival, i kdyz ke konci jen u "jednoradkovych" zalezitosti jako

with TMenuItem(Sender) do
  MenuCode := copy(Name,length(c...)+1,length(Name)-length(c...));

A nekdy od roku 2005, kdy jsem zacal psat vic v Pythonu a Jave jsem na with zanevrel uplne a vubec ho nepouzivam a vubec mi nechybi. A velkou radost by mi udelali, kdy to z prekladace konecne vyfuckovali nebo aspon zavedli option, kterym se to da zakazat.

pf1957

27.8.2016 15:37:11 #

radekc

Pepaku, jako že by došlo k vyjimce během přiřazení Connection? Ano.

radekc

27.8.2016 17:11:26 #

geby

Asi nejsem kazdy pascalista, protoze ja with nikdy dobrovolne nepouzil. Usetri par pismenek, ale zneprehledni kod jak pro vloveka, tak i pro prekladac.

geby

27.8.2016 20:34:18 #

radekc

Já si myslím, že dříve mohlo být with naopak nápovědou pro překladač, že se má pokusit nacpat to co je za with do registru procesoru. Mám to nějak v podvědomí, ale důkaz na to nemám.

radekc

28.8.2016 10:30:07 #

dtoman

drive (no hodne davno) with nad recordem dokazalo usetrit peknych par bytu a cyklu CPU. Cili se ho vyplatilo pouzivat.
To bylo ovsem v dobach, kdy efektivita kodu hrala jeste nejakou roli a kompilator neumel optimalizace a dinosauri prave ulehali do svych bazinatych hrobu..

dtoman

29.8.2016 12:02:40 #

verex

Naprosto stejná zkušenost u převzatého kódu. Záludná chyba kvůli with.

With nemám rád a nepoužívám jej. Max v jednořádkovém užití jak píše pf1957.

verex

29.8.2016 15:49:59 #

bullhead

...chlapi díky ...když jsem se ještě živil programováním, lidé (čti delphi developři v mém okolí) nechápali , že jsem WITH nikdy nepoužil a odmítal jsem to (dle mně je ten kód pak hůře čitelný) ...supr, tak po letech zjišťuji, že je pár lidí, co také ne

B

bullhead

30.8.2016 16:03:37 #

echelon

Také jsem "with" nikdy nepoužíval a domnívám se, že je to snad ještě větší zlo něž "goto".
Myslím, že by bylo vhodné aby Embarcadero každé použití této hrůzy zpoplatnilo alespoň tak $10 a v případě násobného použití bych to mocnil.

echelon

31.8.2016 2:44:33 #

PaJi

Tak koukám, že jsem už asi ten dinosaurus. Já patřím k těm, co "with" sem tam i použijí. Ale zdůrazňuji, že tam, kde to má smysl. Tzn. např. pokud chci inicializovat nějaké recordy defaultními hodnotami apod. Tam se opravdu upsat k smrti nehodlám (většinou extra routina). V kódu, který je v ukázce uveden, to samozřejmě udělá jen "šílenec".

Teda nevím, jak komu, ale mně např. toto přijde přehlednější a jasnější.

begin
  with MyRecord do
  begin
     Val1 := cdefVal1;
     Val2 := cdefVal2;
     Val3 := cdefVal3;
  end;
  with OtherMyRecord do
  begin
     Value1 := cdefValue1;
     Value2 := cdefValue2;
     Value3 := cdefValue3;
  end;
end;

Samozřejmě, tady uvádím pouze pár proměnných, ale pokud jich je větší množství. Ano vyžaduje to od programátora, že ví, co dělá a má určitou sebekázeň. Pokud ji nemá, tak používání nebo nepoužívání with na výsledek moc vliv stejně mít nebude.
Nicméně v objektech je vždy defaultní with (něco jako "with Self do"). Takže to with (i když v objektech skryté) každý v Delphi používá a je jen otázkou, jestli o tom vůbec ví.....

PaJi

31.8.2016 10:41:50 #

pepak

Další IMHO vhodné použití:

with Dataset.FieldByName('nazev') do
  if IsNull then
    Result := ''
  else
    Result := AsString;

pepak

31.8.2016 11:32:26 #

radekc

ad Pepak: no, možná ano, ale spíše zde bych nakonec dal take prednost lokalni promenne TField- už jen pro ladění, kdy tu lokální proměnnou mohu dát do watch (jinak bych musel cely výraz).
Nehledě na to, že možná o deset řádků níže budu přistupovat ke stejnému fieldu, a zase budu volat FieldByName.

radekc

31.8.2016 17:17:33 #

Daniel Andraščík

ad PaJi: Na inicializaciu recordov pouzivam uz tusim od D2007 moznosti nadefinovat si pre record aj vlastne "metody". Cize takmer u kazdeho recordu mam metodu Clear, alebo Init. Vyhodou navyse je ze ak neskor modifikujem datove cleny, tak ma vzdy napadne updatovat spravne aj tie metody Clear, resp. Init a pri mojej discipline sa viem spolahnut na to ze su aktualne a nestane sa mi ze by som na to niekde zabudol, ked mam inicializaciu zasitu niekde idne v programe. Vid. teda nasledujuci priklad:

type
  TMonthStats = record
  private
  public
    MonthDate: TDateTime;
    Profit: Currency;
    Desc: String;

    procedure Clear;
  end;

procedure TMonthStats.Clear;
begin
  MonthDate := 0;
  Profit := 0;
  Desc:= '';
end;

Daniel Andraščík

31.8.2016 19:58:42 #

PaJi

ad Andraščík: No to lze, pokud nejste nucen přebíhat mezi verzemi. Já se switchuju mezi BP7, D5 a Seattlem. Ano u těch novějších lze psát inicializační routiny do recordů, ale budu upřímný, moc jsem na to nepřešel. Také používám inicializační routiny, ale až u objektů, kde předávám sem tam nějaké hodnoty.
Vím, je to starší typ přístupu k věci, ale použijete ho všude - už od dob BP7. Ano u nás jedeme některé věci v DOSu v protected modu. Sice to dožívá, ale je to v cenovém srovnání "nejspolehlivější" systém - u nás pro laboratorní věci. Těch řádově 200 chyb (zejména na INT15) jsou docela dobře popsané a známé. Občas tam chodím poklidit jednou za 2 měsíce, ale jinak vše jede. Wokna musím opečovávat mnohem, mnohem častěji.

Jinak pro zdejší with"bijce", kdopak z Vás tedy píše opravdu plné reference v objektech?

  Self.FMyRec.rVal1 := cdefVal1;
  Self.FMyRec.rVal2 := cdefVal2;

atd., protože jak už jsem dříve uvedl "with Self" je implicitní a v kódu to může nadělat neplechu i tak (pro ty zkušenější, vynechte nyní class reference prosím, jde o pochopení principu).

Tedy pro definice, nebo naplnění ve vybraných routinách jsem používal, používám a asi i budu používat "with" nadále.

PaJi

5.9.2016 10:47:57 #

JaroB.

U metody pro vymazávání záznamů, používá-li někde krátké řetězce nebo pole znaků, je nejlepší používat  vymazání prostoru, aby nezůstala zbytková data takže namísto

desc := '';

bych použil

FillChar(desc, SizeOf(desc), 0);

nebo pro vymazání v metodě Clear()

FillChar(Self, SizeOf(self), 0);

Tohle ale samozřejmě nelze použít, pokud je record občas zaměňován s class např.

type
  TMonthStats = {$IFDEF USE_REC}record{$ELSE}class{$ENDIF} ...

Někdy se to používalo při ladění anebo pro nižší verze non unicode Delphi

JaroB.

15.9.2016 22:13:41 #

PaJi

JaroB:  Tak takhle bych to určitě nedělal, tímto způsobem se zadělává jen na memory leak kvůli neuvolněné paměti. U proměnné "desc" by nedošlo k uvolnění paměti vlastního řetězce (dekrementaci, atd.). Nulování je možné pouze u ordinálních typů. Pokud používáte Variant, Dynamická pole, nebo nějakou odrůdu String (kromě ShortString), tak musíte skutečně nejdříve přiřazovat, aby se dekrementoval čítač a došlo ke správné dealokaci paměti (pokud čítač dojde do nuly).

Máme-li tedy:

TMyRec = packed record
  S1: String;
  I1: LongInt;
  U1: LongWord;
end;

MyRec: TMyRec;

pak čištění musí být takto (zase to berte jako příklad, jde o princip...)

begin
  MyRec.S1 := '';
  FillChar(MyRec, SizeOf(MyRec), 0);
end;

PaJi

19.9.2016 10:58:12 #

JaroB.

To nesouhlasím (píšu speciálně o krátkých řetězcích) - pokud je deklarováno jako TMyRec = packed record - tak string uvnitř, bez uvedené délky, je deklarován jako S1: String[255]; tj jako shortstring a je zbytečné ho extra mazat přiřazením, protože to prostor nesmaže (a je to bezpečnostní incident, pokud se to následně někam ukládá an block).

Naopak, pokud je deklarován pouze jako long string v ne-packed recordu, tak tam je vymazání přiřazením opodstatněné, viz příklad v rozdílu:

type
  TMyPackedRecord = packed record
    S1: string; //je to vlastne ShortString[255]
    D1: Double;
  public
    procedure Clear;
  end;

{ TMyPackedRecord }

procedure TMyPackedRecord.Clear;
begin
  FillChar(Self, SizeOf(Self), 0); //smaze prostor
end;

type
  TMyRecord = record
    S1: string;
    D1: Double;
  public
    procedure Clear;
  end;


{ TMyRecord }

procedure TMyRecord.Clear;
begin
  S1 := ''; // jen promennou
  D1 := 0.0;
end;

JaroB.

20.9.2016 0:12:38 #

PaJi

JaroB:  Nevím, jak ve starších verzích (já skáču z D5 rovnou na D10) ale v 10.0 (Seattle) i v 10.1 (Berlin) bude v TMyPackedRecord S1 vždy v unicode verzi, tzn. že v recordu je uložen pouze pointer na strukturu řetězce, tedy musíte přiřazovat vždy. Slůvko packed dělá pouze to, že "přebíjí" parameter "align", tzn. že proměnné se "salátují" hned za sebou bez ohledu na velikost, kterou zabírají. Není mně známo, že by někdy slovíčko packed měnilo typ (ale vyloučit to samozřejmě nemohu).

  TMyRecPacked = packed record     // SizeOf = 7 Bytes
    r1B: Byte;         // offset = 0
    r1W: Word;      // offset = 1
    r1S: String;      // offset = 3  (pointer na strukturu unicode řetězce)
  end;
  TMyRecNormal = record     // SizeOf = 8 Bytes
    r2B: Byte;         // offset = 0
    r2W: Word;      // offset = 2
    r2S: String;      // offset = 4  (pointer na strukturu unicode řetězce)
  end;

Takže vždy přiřazovat, to platí pro jakékoliv "dynamické" struktury, které jsou v recordu včetně Variant.
Samozřejmě, že pokud je definován ShortString, tak pak není třeba přiřazovat, ale slůvko packed s tím nemá co společného.
V předchozích dvou recordech musíme přiřazovat, pokud však máme definovaný shortstring - viz dále, tak ne:

  TMyRecPackedShort = packed record     // SizeOf = 259 Bytes
    r3B: Byte;         // offset = 0
    r3W: Word;      // offset = 1
    r3S: ShortString;     // offset = 3  (standardní old řetězec - 1 znak délky a 255 jednobytových znaků)
  end;
  TMyRecNormalShort = record     // SizeOf = 260 Bytes
    r3B: Byte;         // offset = 0
    r3W: Word;      // offset = 2
    r3S: ShortString;     // offset = 4  (standardní old řetězec - 1 znak délky a 255 jednobytových znaků)
  end;

Takže správné čištění pak bude:

procedure TMyClass.Clear;
begin
  // je to unicode - r1S je pointer
  MyRecPacked.r1S := '';
  FillChar(MyRecPacked, SizeOf(MyRecPacked), 0);
  // SizeOf je 7 Bytes

  // je to unicode - r2S je pointer
  MyRecNormal.r2S := '';
  FillChar(MyRecNormal, SizeOf(MyRecNormal), 0);
  // SizeOf je 8 Bytes


  // A tady můžeme rovnou (není nutno přiřazovat), neboť jsou vše statické struktury a proměnné:

  FillChar(MyRecPackedShort, SizeOf(MyRecPackedShort), 0);
  // SizeOf je 259 Bytes

  FillChar(MyRecNormalShort, SizeOf(MyRecNormalShort), 0);
  // SizeOf je 260 Bytes
end;

PaJi

20.9.2016 0:19:09 #

radekc

Pro čištění (resp. uvolnění) struktur se dá použít procedura finalize

radekc

20.9.2016 9:14:39 #

JaroB.

Ač nerad, musím připustit, že máte pravdu a já doteď žil v bludu, ohled na packed-nepacked se v tomto případě nebere. Konstrukce "file of TMyPackedRecord" selže. Ověřoval jsem v Seattle.
Bez finalize to funguje jen v případě, když se nadeklaruje řetězec jako S: string[255] tj. jako ekvivalent shortstring.
Kdy došlo k téhle změně netuším, ale schválně jsem prověřil většinu svých projektů za posledních cca deset let - a nikde tuto konstrukci použitou nemám :( kromě řízení záměny record za class.
Omlouvám se za mystifikaci.

JaroB.

21.9.2016 0:49:37 #

PaJi

JaroB: V poho, hlavně že jsme si to ujasnili. Já jsem si aspoň potvrdil, že i "normal" record (tedy NE packed) dělá malou optimalizaci (salátování) - pro mě je to velmi důležité (mám na mysli offset) - tím se liší od "C". Tady jsou změny a ne každá verze Pascalu - Delphi se chová stejně.
Proto mám tyto diskuze rád - občas se něčemu přiučím, potvrdím si, či vyvrátím (nutí to člověka k přemýšlení).

PaJi

21.9.2016 2:17:38 #

radekc

Podívej se na directivu Align - http://docwiki.embarcadero.com/RADStudio/Seattle/en/Align_fields_%28Delphi%29

Určuje zarovnání recordu.

radekc

21.9.2016 9:25:20 #

JaroB.

Jen poznámka, nefunguje to na žádné unicode verzi Delphi.

JaroB.

21.9.2016 9:29:23 #

radekc

Co nefunguje? Align?

radekc

21.9.2016 9:42:46 #

JaroB.

Nefunguje konstrukce, viz (proto je tam podmínka)

type
  TMyPackedRecord = packed record
    S1: string{$IFDEF UNICODE}[255]{$ENDIF};
    D1: Double;
  public
    procedure Clear;
  end;

{ TMyPackedRecord }

procedure TMyPackedRecord.Clear;
begin
  FillChar(Self, SizeOf(Self), 0); //smaze prostor
end;

type
  TMyRecord = record
    S1: string;
    D1: Double;
  public
    procedure Clear;
  end;


{ TMyRecord }

procedure TMyRecord.Clear;
begin
  S1 := ''; // jen promennou
  D1 := 0.0;
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  T: TMyPackedRecord;
  F: file of TMyPackedRecord;
begin
  T.Clear;
  ShowMessage(IntToStr(SizeOf(T)));
  AssignFile(F, 'c:\test.bin');
  Rewrite(F);
  Write(F, T);
  CloseFile(F);
end;

JaroB.

21.9.2016 13:03:06 #

PaJi

RadekC:  Díky, znám - ale jsou to poslední verze a je to spíše zarovnání lokálních proměnných na zásobnících a data segmentu. Díky tomu, že přeskakuji od BP7 přes D5 až do D10, tak to musím v hlavě u každé verze "řešit". Píšu některé věci v assembleru (32b) a nemám možnost si tam přetáhnout record s offsety (můj problém - vím), takže si je musím nadefinovat růčo. Díky tomu, že jsem něco převáděl z D5 do D10, tak jsem právě "narazil". Proto to mám v hlavě zakódovaný a proto ty offsety....

V nových verzích je ještě přepínač "$OLDTYPELAYOUT" (dost umí s rozmístěním v recordu zahýbat podle deklarace). To jsou ty průšvihy s nekompatibilitou s recordy v Céčku. A kdyby to někoho zajímalo, tak popis je spíše tady (podle verze - je to stejné):

http://docwiki.embarcadero.com/RADStudio/Seattle/en/Internal_Data_Formats#Record_Types
http://docwiki.embarcadero.com/RADStudio/Berlin/en/Internal_Data_Formats#Record_Types

PaJi

21.9.2016 14:22:48 #

PaJi

JaroB:  Nevím, co tím míníte, ale v principu to nezáleží na Unicode nebo Ansi, nýbrž na tom, zda-li chci mít statický "shortstring" (který je jen v Ansi verzi), nebo dynamický "string" (který je jak v Ansi, tak i Unicode verzi).

Ansi ShortStringy - STATICKE:
---------------------------------------------
S1: ShortString;
S2: String[20];

DYNAMICKE stringy:
---------------------------------------------
S3: UnicodeString;
S4: AnsiString;
S5: String;
S6: WideString;

-------------------
ShortString = String[255]  - dva různé zápisy téhož.

S dynamickými stringy se dá pak pracovat jako s řetězci ukončenými null = #0, kdežto shortstring si musíme trochu poupravovat. Zejména pro volání API funkcí OS, atd.


Vaše konstrukce ve druhé části nemůže fungovat, neboť dáváte record coby file, ale ten musí být statický, takže pokud tam máte dynamický string, uloží se do souboru jen nesmyslná binární adresa (původně ukazovala na dynamický string). Všechny typy souborů musí být STATICKĚ (jinými slovy musí mít přesně danou velikost, což dynamické proměnné nemají). Proto se to dnes doporučuje dělat přes streamy. Dynamické alokace jsou fajn pro vlastní program z důvodu úspory paměti, ale na soubory se nehodí - tam se musí použít výhradně statika, v tomto případě bych spíše volil nějaké pole WideChar o určité velikosti a k tomu doplácal velmi jednoduché obslužné routiny pro načtení a zápis v kódu programu (zároveň zajistím validaci a velikost). Asi bych to dal přímo jako metody recordu a na vlastní pole jako takové bych "nešahal", využíval bych ho jen jako deklaraci pro daný soubor a buffer.

Jinými slovy, jakmile máte jakoukoliv dynamickou proměnnou v recordu, pak tento record nemůžete použít pro definici souboru - a nijak to nelze obejít. V takovém recordu je možné mít pouze staticky definované prvky, nic víc, nic míň.

PaJi

Komentování ukončeno

Naše nabídka

Partial English version.

MVP
Ing. Radek Červinka - Embarcadero MVP
profil na linkedin, Twitter:@delphicz

Nabízím placené poradenství a konzultace v oblasti programování a vývoje SW.
Dále nabízíme i vývoj speciálního software na zakázku.

Neváhejte nás kontaktovat (i ohledně reklamy nebo burzy práce).

Pokud chcete podpořit tento server libovolnou částkou, můžete použít PayPal. Moc děkuji.

Delphi Certified Developer

O Delphi.cz

Delphi je jediný moderní RAD nástroj podporující tvorbu nativních aplikací pro platformu Win32, Win64 , Mac OSX a na iPhone a Android (s výhledem na další platformy díky FireMonkey) na současném trhu (včetně Windows 8.1).

V současnosti je světová komunita přes dva miliónů vývojářů.

Delphi.cz je nezávislý portál pro uživatele Delphi. Portál není koncipován pro úplné začátečníky, i když i ti se zde nebudou nudit, ale spíše na programátory, kteří již něco znají a chtějí své znalosti dále rozvíjet a sledovat novinky.

Anketa

Poslední komentáře

Comment RSS