View Issue Details

IDProjectCategoryView StatusLast Update
0036886FPCPackagespublic2020-04-08 16:14
ReporterAwkward Assigned ToMichael Van Canneyt  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWindows 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0036886: wrong GetText.pp (fcl-base package) code and example
DescriptionGetText.pp have not only mixed formatting (letter case, tabulations, no "const" before string arguments) but commented line

procedure TranslateUnitResourceStrings(const AUnitName:string; AFile: TMOFile);
begin
// SetUnitResourceStrings(AUnitName,@Translate,AFile);
end;

and don't set value of FallbackLang variable in non-windows GetLanguageIDs function (when environment variable for Loag is empty or not found)

in compiled (release) compiler verison example restest.pp will compile but not work properly because "Intl" directory with test locale files (*.mo and *.po) not presents. and even if it will be present, "make" file for PO to MO translation will not work becouse refers to msgfmt executive which not presented anywhere (and no info where to get it)
TagsNo tags attached.
Fixed in Revision44644
FPCOldBugId
FPCTarget4.0.0
Attached Files

Activities

Awkward

2020-04-08 08:41

reporter   ~0122013

another two things:
1 - "destroy" method uses "Dispose" procedures. but "GetMem" (not "New") was used to reserve memory, so right way will be to use "FreeMem"

2 - Exceptions not used there so next "on e:exception" not necessary
      try
        mo := TMOFile.Create(Format(AFilename, [lang]));
        try
          TranslateResourceStrings(mo);
        finally
          mo.Free;
        end;
      except
        on e: Exception do;
      end;

Anton Kavalenka

2020-04-08 09:50

reporter   ~0122016

https://wiki.freepascal.org/Creating_A_Patch

Michael Van Canneyt

2020-04-08 10:17

administrator   ~0122017

exceptions are used: if an access violation occurs for example. You're dealing with sensitive memory access.
Whether the code should eat the exception is another matter :(

Awkward

2020-04-08 11:10

reporter   ~0122018

Anton Kavalenka, if you read carefully, you can see what not only sourcecode-related problem was there but at least RC installer too

Awkward

2020-04-08 11:17

reporter   ~0122019

This is better? but i modified something for MY coding style :(
gettext.diff (6,786 bytes)   
26c26
<   MOFileHeaderMagic = $950412de;
---
>   MOFileHeaderMagic = $950412DE;
30,33c30,33
<     magic: LongWord;             // MOFileHeaderMagic
<     revision: LongWord;          // 0
<     nstrings: LongWord;          // Number of string pairs
<     OrigTabOffset: LongWord;     // Offset of original string offset table
---
>     magic         : LongWord;    // MOFileHeaderMagic
>     revision      : LongWord;    // 0
>     nstrings      : LongWord;    // Number of string pairs
>     OrigTabOffset : LongWord;    // Offset of original string offset table
35,36c35,36
<     HashTabSize: LongWord;       // Size of hashing table
<     HashTabOffset: LongWord;     // Offset of first hashing table entry
---
>     HashTabSize   : LongWord;    // Size of hashing table
>     HashTabOffset : LongWord;    // Offset of first hashing table entry
56d55
<     StringCount, HashTableSize: LongWord;
58c57,58
<     OrigTable, TranslTable: PMOStringTable;
---
>     StringCount, HashTableSize: LongWord;
>     OrigTable  , TranslTable  : PMOStringTable;
65,66c65,66
<     function Translate(AOrig: String; AHash: LongWord): String;
<     function Translate(AOrig: String): String;
---
>     function Translate(const AOrig: String; AHash: LongWord): String;
>     function Translate(const AOrig: String): String;
72c72
<   procedure GetLanguageIDs(var Lang, FallbackLang: string);
---
>   procedure GetLanguageIDs(var Lang, FallbackLang: String);
74,76c74,76
<   procedure TranslateUnitResourceStrings(const AUnitName:string; AFile: TMOFile);
<   procedure TranslateResourceStrings(const AFilename: String);
<   procedure TranslateUnitResourceStrings(const AUnitName:string; const AFilename: String);
---
>   procedure TranslateUnitResourceStrings(const AUnitName: String; AFile: TMOFile);
>   procedure TranslateResourceStrings    (const AFilename: String);
>   procedure TranslateUnitResourceStrings(const AUnitName: String; const AFilename: String);
87,88c87,88
< procedure Endianfixmotable(p:PMOStringTable;n:integer);
< var I:integer;
---
> procedure EndianFixMOTable(p:PMOStringTable; n:integer);
> var i:integer;
90,95c90,94
<   if n>0 then
<     for i:=0 to n-1 do
<       begin 
<         p^[i].length:=swapendian(p^[i].length);
<         p^[i].offset:=swapendian(p^[i].offset);
<       end;
---
>   for i:=0 to n-1 do
>     begin 
>       p^[i].length:=swapendian(p^[i].length);
>       p^[i].offset:=swapendian(p^[i].offset);
>     end;
98,99c97,98
< procedure Endianfixhashtable(p:PLongwordArray;n:integer);
< var I:integer;
---
> procedure EndianFixHashTable(p:PLongwordArray; n:integer);
> var i:integer;
101,105c100,103
<   if n>0 then
<     for i:=0 to n-1 do
<       begin 
<         p^[i]:=swapendian(p^[i]);
<       end;
---
>   for i:=0 to n-1 do
>     begin 
>       p^[i]:=swapendian(p^[i]);
>     end;
111c109
<   i: Integer;
---
>   i: integer;
123c121
<   If EndianSwap then 
---
>   If endianswap then 
127,128c125,126
<           revision	:=SwapEndian(revision);
<           nstrings	:=SwapEndian(nstrings);
---
>           revision	    :=SwapEndian(revision);
>           nstrings	    :=SwapEndian(nstrings);
136,138c134,136
<   GetMem(OrigTable, header.nstrings * SizeOf(TMOStringInfo));
<   GetMem(TranslTable, header.nstrings * SizeOf(TMOStringInfo));
<   GetMem(OrigStrings, header.nstrings * SizeOf(PChar));
---
>   GetMem(OrigTable    , header.nstrings * SizeOf(TMOStringInfo));
>   GetMem(TranslTable  , header.nstrings * SizeOf(TMOStringInfo));
>   GetMem(OrigStrings  , header.nstrings * SizeOf(PChar));
144,145c142,143
<   if EndianSwap then 
<     EndianFixmotable(OrigTable,Header.NStrings);
---
>   if endianswap then 
>     EndianFixMOTable(OrigTable,Header.NStrings);
149,150c147,148
<   if EndianSwap then 
<     EndianFixmotable(TranslTable,Header.NStrings);
---
>   if endianswap then 
>     EndianFixMOTable(TranslTable,Header.NStrings);
161c159
<     GetMem(OrigStrings^[i], OrigTable^[i].length + 1);
---
>     GetMem      (OrigStrings^[i] , OrigTable^[i].length+1);
172c170
<     GetMem(TranslStrings^[i], TranslTable^[i].length+1);
---
>     GetMem      (TranslStrings^[i] , TranslTable^[i].length+1);
182c180
<   if EndianSwap then 
---
>   if endianswap then 
204,205c202,203
<     Dispose(OrigStrings^[i]);
<     Dispose(TranslStrings^[i]);
---
>     FreeMem(OrigStrings^[i]);
>     FreeMem(TranslStrings^[i]);
207,211c205,210
<   Dispose(OrigTable);
<   Dispose(TranslTable);
<   Dispose(OrigStrings);
<   Dispose(TranslStrings);
<   Dispose(HashTable);
---
>   FreeMem(OrigTable);
>   FreeMem(TranslTable);
>   FreeMem(OrigStrings);
>   FreeMem(TranslStrings);
>   FreeMem(HashTable);
> 
223a223
> 
225a226
> 
234,235c235,237
<     if (OrigTable^[nstr - 1].length = LongWord(ALen)) and
<        (StrComp(OrigStrings^[nstr - 1], AOrig) = 0) then
---
> 
>     if (OrigTable^[nstr-1].length = LongWord(ALen)) and
>        (StrComp(OrigStrings^[nstr-1], AOrig) = 0) then
237c239
<       Result := TranslStrings^[nstr - 1];
---
>       Result := TranslStrings^[nstr-1];
240,241c242,244
<     if idx >= HashTableSize - incr then
<       Dec(idx, HashTableSize - incr)
---
> 
>     if idx >= (HashTableSize-incr) then
>       Dec(idx, HashTableSize-incr)
247c250
< function TMOFile.Translate(AOrig: String; AHash: LongWord): String;
---
> function TMOFile.Translate(const AOrig: String; AHash: LongWord): String;
252c255
< function TMOFile.Translate(AOrig: String): String;
---
> function TMOFile.Translate(const AOrig: String): String;
281c284
< procedure TranslateUnitResourceStrings(const AUnitName:string; AFile: TMOFile);
---
> procedure TranslateUnitResourceStrings(const AUnitName: String; AFile: TMOFile);
283c286
< //  SetUnitResourceStrings(AUnitName,@Translate,AFile);
---
>   SetUnitResourceStrings(AUnitName,@Translate,AFile);
288c291
< procedure GetLanguageIDs(var Lang, FallbackLang: string);
---
> procedure GetLanguageIDs(var Lang, FallbackLang: String);
291c294
<   Country: string;
---
>   Country: String;
300c303,304
<   if GetLocaleInfo(UserLCID, LOCALE_SABBREVCTRYNAME, @Buffer[1], 4)<>0 then begin
---
>   if GetLocaleInfo(UserLCID, LOCALE_SABBREVCTRYNAME, @Buffer[1], 4)<>0 then
>   begin
313c317
< procedure GetLanguageIDs(var Lang, FallbackLang: string);
---
> procedure GetLanguageIDs(var Lang, FallbackLang: String);
322a327,328
>       begin
>         FallbackLang := '';
323a330
>       end;
351a359
> 
370c378
< procedure TranslateUnitResourceStrings(const AUnitName:string; const AFilename: String);
---
> procedure TranslateUnitResourceStrings(const AUnitName: String; const AFilename: String);
gettext.diff (6,786 bytes)   

Michael Van Canneyt

2020-04-08 11:40

administrator   ~0122020

I will check your patch.
But in future please do not change the coding style, it makes it very hard to differentiate actual changes from style changes,

Sven Barth

2020-04-08 11:42

manager   ~0122021

Please leave out style changes as they hide relevant changes.

Michael Van Canneyt

2020-04-08 11:59

administrator   ~0122024

* changed dispose to freemem (although that should not make any difference).
* fixed the translateunitresourcestrings call. (no idea why it was commented out :( )
* added a default to fallbacklang.
* The msgfmt is part of GNU gettext tools. added that to the README.
* If you execute the restest in the fcl-base/examples source directory then it works correctly.

Issue History

Date Modified Username Field Change
2020-04-08 08:10 Awkward New Issue
2020-04-08 08:41 Awkward Note Added: 0122013
2020-04-08 09:50 Anton Kavalenka Note Added: 0122016
2020-04-08 10:16 Michael Van Canneyt Assigned To => Michael Van Canneyt
2020-04-08 10:16 Michael Van Canneyt Status new => assigned
2020-04-08 10:17 Michael Van Canneyt Note Added: 0122017
2020-04-08 11:10 Awkward Note Added: 0122018
2020-04-08 11:17 Awkward File Added: gettext.diff
2020-04-08 11:17 Awkward Note Added: 0122019
2020-04-08 11:40 Michael Van Canneyt Note Added: 0122020
2020-04-08 11:42 Sven Barth Note Added: 0122021
2020-04-08 11:59 Michael Van Canneyt Status assigned => resolved
2020-04-08 11:59 Michael Van Canneyt Resolution open => fixed
2020-04-08 11:59 Michael Van Canneyt Fixed in Version => 3.3.1
2020-04-08 11:59 Michael Van Canneyt Fixed in Revision => 44644
2020-04-08 11:59 Michael Van Canneyt FPCTarget => 4.0.0
2020-04-08 11:59 Michael Van Canneyt Note Added: 0122024
2020-04-08 16:14 Awkward Status resolved => closed