View Issue Details

IDProjectCategoryView StatusLast Update
0032522FPCFVpublic2019-12-30 12:39 Assigned ToMarco van de Voort  
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Summary0032522: FormatStr faulty
DescriptionInstead of 10 to 99, 10 to 1751467531 is outputted.

The number 1751467531 seems to be random, depending on what is in RAM.
The number changes depending on where I start my program.
Steps To Reproduceprogram Project1;
  App, Drivers, MsgBox;

  procedure Test1;
    Params: array[0..1] of longint;
    Params[0] := 10;
    Params[1] := 99;
    MessageBox('Fehler: ' + ' %d to %d', @Params, mfError or mfOKButton);

  procedure Test2;
    s: ShortString;
    Params: array[0..1] of longint;
    p: Pointer;
    Params[0] := 10;
    Params[1] := 99;
    FormatStr(s, 'Fehler: ' + ' %d to %d', p^);

  MyApp: TApplication;

Additional InformationI've noticed the error here:

   InputLine: = new (PInputLine, Init (Rect, 6));
   InputLine ^ .SetValidator (new (PRangeValidator, Init (0, 99)));
   Insert (Input Line);
TagsNo tags attached.
Fixed in Revision
Attached Files


2017-10-07 21:44

reporter   ~0103241

Last edited: 2017-10-07 21:47

View 2 revisions

PS: This problem is only under Linux, under Windows it comes right.

Unit Validate row 920:

PROCEDURE TRangeValidator.Error;
CONST PXErrMsg = 'Value not in the range';
VAR Params: Array[0..1] Of Longint;
   Params[0] := Min; { Transfer min value }
   Params[1] := Max; { Transfer max value }
   MessageBox(PXErrMsg+' %d to %d', @Params,
     mfError OR mfOKButton); { Display message }

2017-10-09 17:57

reporter   ~0103292

Last edited: 2017-10-09 18:04

View 2 revisions

I have further tested.

Win32 Ok
Win64 error
Linux32 Ok
Linux64 error

So it is at the 64bit and not at Linux.

I declare Params as an int64 array, then it works with 64Bit, but 32Bit is no longer.

  procedure Test2;
    s: ShortString;
    Params: array[0..5] of int64;
    p: Pointer;

2017-10-09 18:12

reporter   ~0103293

Ändere ich die Source ab, scheint es auf allen 4 OS zu funktionieren.

procedure FormatStr (Var Result: ShortString; CONST Format: ShortString; Var Params);
//TYPE TLongArray = Array[0..0] Of PtrInt;
TYPE TLongArray = Array[0..0] Of Int32;

Das Problem liegt daran, das PtrInt bei 64Bit Int64 ist und bei 32Bit ein LongInt, siehe ab Zeile 370

Marco van de Voort

2017-10-09 18:59

manager   ~0103299

Yeah, but if it isn't ptrint, %s probably is a problem:

 's': S := PString(TLongArray(Params)[I])^;{ String parameter }

So my guess is that should be documented that the type should match the pointer size. Even then it is so dangerous that imho it is better to deprecate the procedure

2017-10-09 20:08

reporter   ~0103302

I have still tested the following, this seems to work with 32 and 64Bit.
Or have I overlooked something?

  TPStringArray = Array[0..0] Of PtrInt;
  TLongArray = Array[0..0] Of Int32;
             'c': S := Char(TLongArray(Params)[I]); { Character parameter }
             'd': S := LongToStr(TLongArray(Params)[I],
               10); { Decimal parameter }
// 's': S := PString(TLongArray(Params)[I])^;{ String parameter }
             's': S := PString(TPStringArray(Params)[I])^;{ String parameter }
             'x': S := LongToStr(TLongArray(Params)[I],
               16); { Hex parameter }

Marco van de Voort

2017-10-10 21:16

manager   ~0103343

Do you use %s and %d in the same format string ? Where is your test?

2017-10-14 18:21


formatstr_bug.tar.gz (3,841 bytes)

2017-10-14 18:30

reporter   ~0103428

Formatting with Char does not work even in 32Bit mode.

  procedure TestChar;
    s: string;
    Params: array[0..5] of char;
    p: Pointer;
    Params[0] := 'a';
    Params[1] := 'b';
    Params[2] := 'c';
    Params[3] := 'd';
    Params[4] := 'e';
    Params[5] := 'f';
    p := @Params;
    FormatStr(s, 'Char: ' + ' %c - %c - %c - %c - %c - %c', p^);

In appendix I have a modified version of FormatStr, so it seems this works also with 64Bit and with a mixture of %d and %s. Provided you declare the record as "packed record".
 %c does not go with 64Bit as expected.

Marco van de Voort

2017-10-15 19:11

manager   ~0103449

Format strings shouldn't just use one type (%c, %s ,%d) but a mix.

The most logic would probably to just use array of ptr(u)int, and the read value according to % modifier.

2017-10-15 19:58

reporter   ~0103451

>Format strings shouldn't just use one type (%c, %s ,%d) but a mix.

Did you look at the file in the Attached Files synonymous?


2017-10-15 22:27

administrator   ~0103454

>>Format strings shouldn't just use one type (%c, %s ,%d) but a mix.

> Did you look at the file in the Attached Files synonymous?

The patch does not solve this problem. I think the only clean solution is to require that FormatStr gets an array ... of PtrInt. Everything else is very very error prone.

Marco van de Voort

2017-10-15 22:45

manager   ~0103455

Last edited: 2017-10-16 10:40

View 3 revisions

> Did you look at the file in the Attached Files synonymous?

Yes, but it has a different interface. If we require existing code to be changed (other than change the base array type), we might as well point people to sysutils.format.

Giving the new type a good name (like PFormatStrArgumentArray) or so might also help.

2017-10-16 22:02

reporter   ~0103495

I have two digits in the FreeVision code, where FormatStr works under 64Bit incorrectly

This is for:
PRangeValidator (TRangeValidator.Error;) (FUNCTION MessageBoxRectDlg)
PIndicator (TIndicator.Draw;)

the case.

If you would patch these functions, you would be the least.

>Der Patch löst dieses Problem nicht. Ich denke, die einzige saubere Lösung besteht darin zu verlangen, dass FormatStr ein Array ... von PtrInt bekommt. Alles andere ist sehr sehr fehleranfällig.

For my own applications I would find this a good solution, unless you ported a program of Turbo-Pascal


2019-01-09 21:22

reporter   ~0113292

Last edited: 2019-01-09 21:29

View 2 revisions

related to 0034839
function FormatStr parameter Params should be defined as array of PtrInt or PtrUInt or Pointer.
That apply also to function MessageBox as it use FormatStr.

uses drivers;
var a :array [0..3] of PtrInt;
    s1, s2, s3 : string;
     a[0]:=PtrInt(@s1); {string}
     a[1]:=byte('@'); {char}
     a[2]:=1; {longint}
     a[3]:=$0abc; {longint}

     s2:='%%foo%% %s %c from %d to 0x%x';

     formatStr(s3,s2, a);
%foo% bar @ from 1 to 0xABC

suggestion close with resolution: no changes required

Marco van de Voort

2019-01-10 09:35

manager   ~0113297

I agree, if there are still problems remaining with current sources, please reopen

Issue History

Date Modified Username Field Change
2017-10-07 21:42 New Issue
2017-10-07 21:44 Note Added: 0103241
2017-10-07 21:47 Note Edited: 0103241 View Revisions
2017-10-09 17:57 Note Added: 0103292
2017-10-09 18:04 Note Edited: 0103292 View Revisions
2017-10-09 18:12 Note Added: 0103293
2017-10-09 18:59 Marco van de Voort Note Added: 0103299
2017-10-09 20:08 Note Added: 0103302
2017-10-10 21:16 Marco van de Voort Note Added: 0103343
2017-10-14 18:21 File Added: formatstr_bug.tar.gz
2017-10-14 18:30 Note Added: 0103428
2017-10-15 19:11 Marco van de Voort Note Added: 0103449
2017-10-15 19:58 Note Added: 0103451
2017-10-15 22:27 Florian Note Added: 0103454
2017-10-15 22:45 Marco van de Voort Note Added: 0103455
2017-10-16 10:40 Marco van de Voort Note Edited: 0103455 View Revisions
2017-10-16 10:40 Marco van de Voort Note Edited: 0103455 View Revisions
2017-10-16 22:02 Note Added: 0103495
2019-01-09 21:22 Marģers Note Added: 0113292
2019-01-09 21:29 Marģers Note Edited: 0113292 View Revisions
2019-01-10 09:35 Marco van de Voort Note Added: 0113297
2019-01-10 09:35 Marco van de Voort Status new => resolved
2019-01-10 09:35 Marco van de Voort Resolution open => fixed
2019-01-10 09:35 Marco van de Voort Assigned To => Marco van de Voort