View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0037564 | Lazarus | Packages | public | 2020-08-13 11:26 | 2020-12-27 13:04 |
| Reporter | CudaText man_ | Assigned To | Bart Broersma | ||
| Priority | normal | Severity | minor | Reproducibility | N/A |
| Status | closed | Resolution | no change required | ||
| Product Version | 2.1 (SVN) | ||||
| Summary | 0037564: Proposal: 2 funcs BufferStrToInt, BufferHexToInt | ||||
| Description | I made 2 funcs for CudaText and suggest to add em in some Lazarus unit. One of em can be used to optimize fpDebug here:
</home/user/lazarus/components/fpdebug/fpdbgrsp.pas>: 0000004
<( 458:19:12)>: result := StrToInt('$' + copy(msg, 2, 2));
<( 636:15:12)>: b[i] := StrToInt('$'+reply[2*i+1]+reply[2*i+2]);
<( 687:17:12)>: buf[i] := StrToInt('$'+reply[2*i + 1]+reply[2*i + 2])
I post funcs to the 1st comment to edit it later. | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | |||||
| LazTarget | - | ||||
| Widgetset | |||||
| Attached Files |
| ||||
|
|
function BufferHexToInt(p: PChar; Len: integer): integer; //Alexey
var
N, i: integer;
ch: Char;
begin
Result:= 0;
for i:= 1 to Len do
begin
ch:= p^;
case ch of
'0'..'9':
N:= Ord(ch)-Ord('0');
'a'..'f':
N:= Ord(ch)-(Ord('a')-10);
'A'..'F':
N:= Ord(ch)-(Ord('A')-10);
else
exit(-1);
end;
Inc(p);
Result:= Result*16+N;
end;
end;
function BufferStrToInt(p: PChar; Len: integer): integer; //Alexey
var
N, i: integer;
ch: Char;
begin
Result:= 0;
for i:= 1 to Len do
begin
ch:= p^;
if ch in ['0'..'9'] then
N:= Ord(ch)-Ord('0')
else
exit(-1);
Inc(p);
Result:= Result*10+N;
end;
end;
|
|
|
And 3rd one
function BufferHexToInt(const S: string; Index, Len: integer): integer;
var
N, i: integer;
ch: char;
begin
Result:= 0;
for i:= Index to Min(Index+Len-1, Length(S)) do
begin
ch:= S[i];
case ch of
'0'..'9':
N:= Ord(ch)-Ord('0');
'a'..'f':
N:= Ord(ch)-(Ord('a')-10);
'A'..'F':
N:= Ord(ch)-(Ord('A')-10);
else
exit(-1);
end;
Result:= Result*16+N;
end;
end;
|
|
|
Can be used here too
</home/user/lazarus/components/lazdebuggers/lazdebuggerlldb/lldbdebugger.pas>: 0000001
<( 765:14:15)>: l := StrToIntDef('$'+copy(s, i+1, 2), 255);
</home/user/lazarus/lcl/propertystorage.pas>: 0000001
<( 201:14:15)>: C := Chr(StrToIntDef('$' + Copy(Source, (I * 2) + 1, 2), Ord(' ')));
|
|
|
Can be used here
+Search for "StrToInt\w*\s*\(\s*copy" in "*.pas *.pp *.inc" from "/home/user/lazarus" (88 matches in 43(3780) files)
</home/user/lazarus/tools/iconvtable_dbcs.pas>: 0000002
<( 203:13:16)>: DBCS:=StrToIntDef(copy(s,1,p-1),0);
<( 204:12:16)>: UTF:=StrToIntDef(copy(s,p+1,10),0);
</home/user/lazarus/lcl/lclproc.pas>: 0000001
<( 2459:23:16)>: +IntToStr(1+StrToIntDef(copy(Identifier,p+1,length(Identifier)-p),0));
</home/user/lazarus/lcl/editbtn.pas>: 0000002
<( 1699: 7:16)>: N1:=StrToIntDef(Copy(S,1,P-1),-1);
<( 1705: 7:16)>: N2:=StrToIntDef(Copy(S,1,P-1),-1);
</home/user/lazarus/lcl/include/tiffimage.inc>: 0000006
<( 42:10:13)>: if TryStrToInt(Copy(S, 1, 4), YY)
<( 43:10:13)>: and TryStrToInt(Copy(S, 6, 2), MM)
<( 44:10:13)>: and TryStrToInt(Copy(S, 9, 2), DD)
<( 45:10:13)>: and TryStrToInt(Copy(S, 12, 2), HH)
<( 46:10:13)>: and TryStrToInt(Copy(S, 15, 2), NN)
<( 47:10:13)>: and TryStrToInt(Copy(S, 18, 2), SS)
</home/user/lazarus/lcl/interfaces/qt5/qtint.pp>: 0000003
<( 516:10:13)>: TryStrToInt(Copy(S, 1, i -1), AMajor);
<( 523:10:13)>: TryStrToInt(Copy(S, 1, i -1), AMinor);
<( 529:10:13)>: TryStrToInt(Copy(S, 1, i -1), AMinor);
</home/user/lazarus/lcl/interfaces/qt/qtint.pp>: 0000003
<( 517:10:13)>: TryStrToInt(Copy(S, 1, i -1), AMajor);
<( 524:10:13)>: TryStrToInt(Copy(S, 1, i -1), AMinor);
<( 530:10:13)>: TryStrToInt(Copy(S, 1, i -1), AMinor);
</home/user/lazarus/converter/convcodetool.pas>: 0000001
<( 452:15:13)>: xNum:=StrToInt(copy(aStr, xStart+1, xLen-1)); // Leave out '$', convert number.
</home/user/lazarus/converter/missingpropertiesdlg.pas>: 0000002
<( 161: 8:13)>: c:=StrToInt(Copy(InS, Start, Ind-Start));
<( 497:17:13)>: OldNum:=StrToInt(Copy(fLFMBuffer.Source, TopOffs.StartPos, Len));
</home/user/lazarus/components/lazutils/translations.pas>: 0000001
<( 1350:29:13)>: Ch := Chr(StrToInt(copy(Line, j, p-j)));
</home/user/lazarus/components/codetools/basiccodetools.pas>: 0000001
<( 5445:19:16)>: Number:=StrToIntDef(copy(Src,NumberStart,APos-NumberStart),-1);
</home/user/lazarus/components/codetools/pascalreadertool.pas>: 0000001
<( 1844:21:16)>: Number:=StrToIntDef(copy(Src,NumberStart-PChar(Src)+1,p-NumberStart),-1);
</home/user/lazarus/components/codetools/codetoolgdbtracer.pas>: 0000001
<( 485:16:16)>: GDBLine:=StrToIntDef(copy(Source,StartP-PChar(Source)+1,p-StartP),0);
</home/user/lazarus/components/codetools/codecache.pas>: 0000001
<( 731:11:16)>: Days:=StrToIntDef(copy(IncludedByFile,p+1,length(IncludedByFile)),0);
</home/user/lazarus/components/codetools/lfmtrees.pas>: 0000001
<( 1033:10:16)>: i:=StrToIntDef(copy(Src,AtomStart+1,p-AtomStart-1),-1);
</home/user/lazarus/components/wiki/lazwiki/wiki2xhtmlconvert.pas>: 0000004
<( 485:22:13)>: h := StrToInt(Copy(s, 2, Length(s)-3))
<( 489:24:13)>: w := StrToInt(Copy(s, 1, Length(s)-2))
<( 493:24:13)>: w := StrToInt(copy(s, 1, p-1));
<( 494:24:13)>: h := StrToInt(copy(s, p+1, Length(s)-p-2));
</home/user/lazarus/components/lazreport/source/lr_class.pas>: 0000003
<(10062:40:13)>: AFormat := AFormat + 256 * StrToInt(Copy(AFormatStr, j, i - j));
<(11515:15:13)>: n2 := StrToInt(Copy(s, j, i - j));
<(11533:20:13)>: n1 := StrToInt(Copy(s, j, i - j));
</home/user/lazarus/components/lazreport/source/lr_view.pas>: 0000001
<( 543:17:16)>: PP:=StrToIntDef(Copy(AInfo, K+1, 255), -1);
</home/user/lazarus/components/lazreport/source/lr_edit.pas>: 0000002
<( 298: 8:13)>: Y := StrToInt(Copy2SymbDel(S, '/'));
<( 299: 8:13)>: X := StrToInt(Copy2SymbDel(S,':'));
</home/user/lazarus/components/lazreport/source/barcode.pas>: 0000001
<( 1050:25:16)>: tmp := tmp + chr( StrToIntDef(Copy(FCodeText, i, 2), 0) );
</home/user/lazarus/components/fpdebug/fppascalparser.pas>: 0000001
<( 1868:18:16)>: c := StrToIntDef(copy(p , 1 , TokenEndPtr - p), -1);
</home/user/lazarus/components/fpdebug/fpdbgdwarffreepascal.pas>: 0000003
<( 268:12:16)>: j := StrToIntDef(copy(s, 1, i - 1), 0);
<( 276:14:16)>: j := StrToIntDef(copy(s, 1, i - 1), 0);
<( 287:14:16)>: j := StrToIntDef(copy(s, 1, i - 1), 0);
</home/user/lazarus/components/buildintf/packagedependencyintf.pas>: 0000001
<( 191:13:16)>: V := StrToIntDef(Copy(Version, 1, P-1), 0);
</home/user/lazarus/components/tachart/tahtml.pas>: 0000002
<( 107:17:13)>: Result := StrToInt(Copy(AText, 1, Length(AText) - 2))
<( 111:17:13)>: Result := StrToInt(Copy(AText, 1, Length(AText) - 2));
</home/user/lazarus/components/turbopower_ipro/ipcss.inc>: 0000003
<( 532:18:16)>: Result.Size:=StrToIntDef(copy(S,1,i-1),0);
<( 539:18:16)>: Result.Size:=StrToIntDef(copy(S,1,i-1),0);
<( 546:18:16)>: Result.Size:=StrToIntDef(copy(S,1,i-1),0);
</home/user/lazarus/components/IdeInspector/maininspector.pas>: 0000001
<( 300:25:16)>: FCurEntry.Line := StrToIntDef(copy(s2, 1, i), -1);
</home/user/lazarus/components/lazdebuggers/lazdebuggerlldb/lldbdebugger.pas>: 0000001
<( 857:15:16)>: Result := StrToIntDef(copy(AReason, 12, i-12), -1);
</home/user/lazarus/components/lazdebuggers/lazdebuggerlldb/lldbhelper.pas>: 0000001
<( 194:16:16)>: ALine := StrToIntDef(copy(found[1], 1, i-1), 0);
</home/user/lazarus/components/lazdebuggers/lazdebuggerlldb/lldbinstructions.pas>: 0000001
<( 1173:18:16)>: FRes[i] := StrToIntDef(copy(s,1,4), 0);
</home/user/lazarus/components/leakview/leakinfo.pas>: 0000002
<( 556:18:18)>: line.Addr := StrToInt64Def(copy(s,1,i-1), 0);
<( 571:21:16)>: line.LineNum := StrToIntDef(copy(s,1,i-1), 0);
</home/user/lazarus/components/synedit/syneditkeycmds.pp>: 0000001
<( 718:12:16)>: Cmd := StrToIntDef(copy(Ident, 14, length(Ident)), -1) + ecUserDefinedFirst;
</home/user/lazarus/components/ideintf/idewindowintf.pas>: 0000005
<( 1314:15:13)>: SubIndex := StrToInt(copy(Result, i+1, length(Result)));
<( 1956:42:16)>: DefBounds.Left:=ScreenR.Left+ScreenW*StrToIntDef(copy(Left,1,length(Left)-1),0) div 100
<( 1963:40:16)>: DefBounds.Top:=ScreenR.Top+ScreenH*StrToIntDef(copy(Top,1,length(Top)-1),0) div 100
<( 1977:23:16)>: aRight:=ScreenW*StrToIntDef(copy(Right,1,length(Right)-1),0) div 100
<( 1999:24:16)>: aBottom:=ScreenH*StrToIntDef(copy(Bottom,1,length(Bottom)-1),0) div 100
</home/user/lazarus/components/ideintf/propedits.pp>: 0000002
<( 3809:38:13)>: if NewValue[1] = '#' then L := StrToInt(Copy(NewValue, 2, Maxint)) else
<( 7557:18:16)>: Result:=word(StrToIntDef(copy(s,7,length(s)-8),VK_UNKNOWN));
</home/user/lazarus/components/lazsvnpkg/svnclasses.pas>: 0000006
<( 257: 8:13)>: y := StrToInt(Copy(ADateTime, 1, 4));
<( 258: 8:13)>: m := StrToInt(Copy(ADateTime, 6, 2));
<( 259: 8:13)>: d := StrToInt(Copy(ADateTime, 9, 2));
<( 260: 8:13)>: h := StrToInt(Copy(ADateTime, 12, 2));
<( 261: 8:13)>: n := StrToInt(Copy(ADateTime, 15, 2));
<( 262: 8:13)>: s := StrToInt(Copy(ADateTime, 18, 2));
</home/user/lazarus/components/lazdebuggergdbmi/gdbmidebugger.pp>: 0000005
<( 3299:13:16)>: AnId := StrToIntDef(copy(AText, j, i2-j), -1);
<( 3467:38:16)>: FTheDebugger.FGDBVersionMajor := StrToIntDef(copy(s,1,i-1), -1);
<( 3474:38:16)>: FTheDebugger.FGDBVersionMinor := StrToIntDef(copy(s,1,i-1), -1);
<( 3482:36:16)>: FTheDebugger.FGDBVersionRev := StrToIntDef(copy(s,1,i-1), -1);
<(11794:14:16)>: t := StrToIntDef(copy(s, 1+length(OptTimeout), MaxInt), DefaultTimeOut);
</home/user/lazarus/components/lazdebuggergdbmi/test/testdisass.pas>: 0000001
<( 89:12:16)>: num := StrToIntDef(copy(FTestCmdLine, p1, pos-p1), -1);
</home/user/lazarus/components/PascalScript/Source/uPSRuntime.pas>: 0000002
<( 7085:20:13)>: ct := FTypes[StrToInt(copy(GRLW(s), 2, MaxInt))];
<( 7134:20:13)>: ct := FTypes[StrToInt(copy(GRLW(s), 2, MaxInt))];
</home/user/lazarus/ide/ideprocs.pp>: 0000002
<( 1305:12:16)>: Point.X:=StrToIntDef(copy(s,1,p-1),DefaultPoint.X);
<( 1306:12:16)>: Point.Y:=StrToIntDef(copy(s,p+1,length(s)-p),DefaultPoint.Y);
</home/user/lazarus/ide/editormacrolistviewer.pas>: 0000002
<( 926:27:13)>: FParams[c].Num := StrToInt(copy(FText, j, i-j));
<( 945:22:13)>: k := StrToInt(copy(FText, j, i-j));
</home/user/lazarus/ide/sourceeditor.pp>: 0000001
<(10126: 8:16)>: i := StrToIntDef(copy(aFormName, i, MaxInt), 1)-1;
</home/user/lazarus/test/testresult-db/importtestresults.pp>: 0000005
<( 136:18:13)>: year:=StrToInt(Copy(value,1,4));
<( 137:19:13)>: month:=StrToInt(Copy(value,5,2));
<( 138:17:13)>: day:=StrToInt(Copy(Value,7,2));
<( 139:18:13)>: hour:=StrToInt(Copy(Value,9,2));
<( 140:17:13)>: min:=StrToInt(Copy(Value,11,2));
</home/user/lazarus/test/testresult-db/proctestsuitediff.pp>: 0000001
<( 203:20:16)>: curr.hour := strtointdef(copy(curr.line, houroffset, 2), 0);
</home/user/lazarus/packager/packagelinks.pas>: 0000001
<( 416:16:16)>: ints[i]:=StrToIntDef(copy(Filename,StartPos,EndPos-StartPos),-1);
</home/user/lazarus/examples/fontenum/mainunit.pas>: 0000001
<( 337:17:13)>: result := StrToInt(Copy(S, 1, i-1))
|
|
|
IIUC then your proposed functions return -1 upon failure, but then you propose to replace several pieces of code that return zero upon failure? I fail to see what problem you code is solving. |
|
|
This is ridiculous. I see no value at all bloating up the apps with this.. |
|
|
Function must be fixed to return 0 upon failure. I must fix it? |
|
|
A function for general use that converts (part of a) string (or PChar) to an integer value should not return a value if the function fails. This is congruent with StrToInt etc. These will raise an exception upon failure. IMO there are 3 options: 1. Make it like TryStrToInt() and family. Return True on succes, False on failure, have an out parameter for the result of conversion. 2. Make it like StrToIntDef() and family, so that the user can provide a default value when conversion fails. 3. Don't make it a function for general use, but in that case IMO there is little gain in using it. And it has the possibility to introduce new bugs in existing, well tested, code. |
|
|
> And it has the possibility to introduce new bugs in existing, well tested, code. Your functions can trigger an EIntOverflow, whereas StrToIntDef would not in such a case:
S := '99999999999999999999999999999999999999999999999999999999999999';
N := StrToIntDef(S,-1234);
writeln('N = ',N); //gives -1234
N := BufferStrToInt(PChar(S), Length(S)); //EIntOverflow: Arithmetic overflow
writeln('N = ',N); |
|
|
Please close. |
|
|
program Project1; uses sysutils; function BufferHexToInt(const S: string; Index, Len: integer): integer; var N, i, up: integer; begin Result:= 0; if Index+Len-1>Length(S) then up:=Length(s) else up:=index+len-1; for i:= Index to up do begin case s[i] of '0'..'9':N:= Ord(s[i])-Ord('0'); else exit(0); end; Result:= Result*10+N; end; end; procedure testcopy; var s: String; p, k, v: LongInt; i: Integer; tm: LongWord; begin s:='11-02-1999'; tm:=GetTickCount; for i:=0 to 1000000 do begin p:=strtoint(Copy(s, 1, 2)); k:=strtoint(Copy(s, 4, 2)); v:=strtoint(Copy(s, 7, 4)); end; writeln('copy speed ',GetTickCount-tm); end; procedure testbuffer; var s: String; p, k, v: LongInt; i: Integer; tm: LongWord; begin s:='11-02-1999'; tm:=GetTickCount; for i:=0 to 1000000 do begin p:=BufferHexToInt(s, 1, 2); k:=BufferHexToInt(s, 4, 2); v:=BufferHexToInt(s, 7, 4); end; writeln('buffer speed ',GetTickCount-tm); end; begin testcopy; testbuffer; end. Output:: copy speed 331 buffer speed 23 I see good intention here. Creating temp string is inefficient. If one needs fast implementation he needs to write code himself. Why Pascal is so slow compared to C? |
|
|
Disregarding all safeguards will make your code faster, not necessarily better. The proposal was to use this in various places in Lazarus: exchanging well tested code with dangerous code. Note: Adding notes to a bugreport with status "resolved" have a high likelyhood of not getting noticed at all. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2020-08-13 11:26 | CudaText man_ | New Issue | |
| 2020-08-13 11:26 | CudaText man_ | Note Added: 0124827 | |
| 2020-08-13 11:27 | CudaText man_ | Note Edited: 0124827 | View Revisions |
| 2020-08-13 11:36 | CudaText man_ | Note Edited: 0124827 | View Revisions |
| 2020-08-13 12:03 | CudaText man_ | Note Added: 0124831 | |
| 2020-08-13 12:58 | CudaText man_ | Note Added: 0124833 | |
| 2020-08-13 12:59 | CudaText man_ | Note Edited: 0124833 | View Revisions |
| 2020-08-13 13:01 | CudaText man_ | Note Added: 0124834 | |
| 2020-11-14 17:05 | Bart Broersma | Note Added: 0126934 | |
| 2020-11-14 21:00 | jamie philbrook | Note Added: 0126941 | |
| 2020-11-14 22:30 | CudaText man_ | Note Added: 0126945 | |
| 2020-11-14 22:37 | Bart Broersma | Note Added: 0126947 | |
| 2020-11-16 22:53 | Bart Broersma | Note Added: 0126989 | |
| 2020-11-22 16:47 | Bart Broersma | Assigned To | => Bart Broersma |
| 2020-11-22 16:47 | Bart Broersma | Status | new => resolved |
| 2020-11-22 16:47 | Bart Broersma | Resolution | open => no change required |
| 2020-11-22 16:47 | Bart Broersma | LazTarget | => - |
| 2020-11-22 16:47 | Bart Broersma | Note Added: 0127109 | |
| 2020-11-22 19:06 | delfion | Note Added: 0127119 | |
| 2020-11-22 23:09 | Bart Broersma | Note Added: 0127125 | |
| 2020-12-27 13:04 | CudaText man_ | Status | resolved => closed |
