GetLocaleInfo() Aufruf, ist es so richtig?

[himitsu]

Zitat von KodeZwerg:

Hallo, ich würde mich gerne absichern ob folgender Code korrekt ist
...
Oder ist es generell so falsch und es gibt aktuellere Methoden um das Zeichen (als String) zu bekommen?

Gegenfrage: Warum hörst du nicht auf deinen Compiler? (vonwegen Variable nicht initialisiert)

buf := StrAlloc(10); // puffer eine größe zusichern gehört vor das TRY
und Result := StrPas(buf); gehört vor Finally oder hinter das End.

Falls StrAlloc knallt, führst du dennoch StrDispose aus, es knallt nochmal und verfälscht den eigentlichen Fehler
und wenn es knallt, greifst du auch noch auf buf zu, was ebenfalls nicht gut ist.

UND, diese API löst keine Exceptions aus,
also hilft Try-Finally/Except garnichts.
Und falls es doch knallt, dann hattest DU "buf" falsch/nicht initialisiert
oder es ist so viel kaputt, dass eine Fehlerbehandlung eh sinnlos ist. (ist = war vorher schon)

Zitat:

falsch initialisieren, gegebenfalls später mit einem Default-Wert bei Fehler ersetzen

Und wie willst du erkennen, ob es einen Fehler gab, wenn du das Result dieser API nicht auswertest?


PS: Wenn ich Code als "Delphi/Pascal" implementiere, dann lasse ich das C-typische StrAlloc weg und nutzte einen passenden Delphi-String (AnsiString/UnicodeString/String)
und das StrDispose macht Delphi für mich.
Nach der API mit SetLength oder einem billigen PChar-Cast den String kürzen, falls der Inhalt doch kleiner war.

Oder ich nutze ein statisches Char-Array und SetString

[KodeZwerg]

So war mein Ansatz nach Zacherl's erstem Post:

Delphi-Quellcode:

function ThousandSeparator : String;
var
  buf: PChar;
begin
  Result := ','; // falls was schiefläuft einen default Wert
  buf := StrAlloc(10);
  if GetLastError() = ERROR_SUCCESS then
   if GetLocaleInfo(
       LOCALE_SYSTEM_DEFAULT,
       LOCALE_STHOUSAND,
       buf,
       10) <> 0 then Result := StrPas(buf);
   if GetLastError() <> ERROR_INSUFFICIENT_BUFFER then StrDispose(buf);
end;

[Zacherl]

Nee das ist auch Murks In etwa so müsste es aussehen:

Delphi-Quellcode:

var
  Buffer: PChar;
  BufferLen: Integer;
begin
  BufferLen := GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, nil, 0);
  if (BufferLen = 0) then
  begin
    RaiseLastOSError;
  end;
  Buffer := StrAlloc(BufferLen);
  if (not Assigned(Buffer)) then
  begin
    raise EOutOfMemory.Create('StrAlloc failed');
  end;
  try
    if (GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, Buffer, BufferLen) <> BufferLen) then
    begin
      RaiseLastOSError;
    end;
    Result := StrPas(Buffer);
  finally
    StrDispose(Buffer);
  end;
end;

[günni0]

Was soll bei  Result := FormatSettings.ThousandSeparator; denn schiefgehen?

[Zacherl]

Zitat von günni0:

Was soll bei  Result := FormatSettings.ThousandSeparator; denn schiefgehen?

Nichts. Wenn man die gekapselte Version nimmt, ist man so ziemlich auf der sicheren Seite. Das "Schiefgehen" bezog sich auf den WinAPI Ansatz bzw. insbesondere das StrDispose im Falle eines vorher erfolglosen StrAlloc s.

[himitsu]

Zitat:

Delphi-Quellcode:

  buf := StrAlloc(10);
  if GetLastError() = ERROR_SUCCESS then

Was soll GetLastError liefern, wenn StrAlloc keinen Fehler lieferte?

Etwas Zufälliges und zwar "laut Definition" den Fehlercode irgendeiner WinAPI, welches davor als Letztes einen Fehler lieferte.


Genau darum steht auch niemals "Ergebnis siehe GetLastError,
sondern "prüfe Result und wenn was schief lief, dann siehe GetLastError".

Zitat:

Delphi-Quellcode:

if GetLocaleInfo(
       LOCALE_SYSTEM_DEFAULT,
       LOCALE_STHOUSAND,
       buf,
       10) <> 0 then Result := StrPas(buf);
   if GetLastError() <> ERROR_INSUFFICIENT_BUFFER then

Wenn GetLocaleInfo erfolgreich war, dann war in StrPas die letzte WinAPI, aber vielleicht kommt das Result auch von irgendeiner WinAPI von irgendwann weit davor.

[KodeZwerg]

Danke nochmals für die korrekte Handhabung per Api @Zacherl, ich hatte den Ansatz eigentlich nur gepostet wegen voriger Aussage warum ich nicht auf Compiler-Meldungen achte, das hatte ich da auch wenn Code falsch ist

[KodeZwerg]

Könnte man Dein Beispiel auch so abändern oder ginge dabei wieder etwas schief?

Delphi-Quellcode:

function ThousandSeparator : String;
const
 Default = ','; // Bei Fehler ein Default Wert nutzen um Meldungen zu überspringen
var
  Buffer: PChar;
  BufferLen: Integer;
begin
  BufferLen := GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, nil, 0);
  if (BufferLen = 0) then
  begin
    Result := Default;
    System.SetLastError(ERROR_SUCCESS);
    Exit;
    // RaiseLastOSError;
  end;
  Buffer := StrAlloc(BufferLen);
  if (not Assigned(Buffer)) then
  begin
    Result := Default;
    System.SetLastError(ERROR_SUCCESS);
    Exit;
    // raise EOutOfMemory.Create('StrAlloc failed');
  end;
  try
    if (GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_STHOUSAND, Buffer, BufferLen) <> BufferLen) then
    begin
      Result := Default;
      if Assigned(Buffer) then StrDispose(Buffer);
      System.SetLastError(ERROR_SUCCESS);
      Exit;
      // RaiseLastOSError;
    end;
    Result := StrPas(Buffer);
  finally
    StrDispose(Buffer);
  end;
end;

[Zacherl]

Würde soweit funktionieren. Die extra Abfrage bezüglich String freigeben kannst du dir aber sparen, da der finally Abschnitt in jedem Falle ausgeführt wird (selbst bei Exit ). Und SetLastError lass am besten auch komplett weg.

[KodeZwerg]

Vielen Dank! Ich habe nun das Extra StrDispose() und die SetLastError() Aufrufe entfernt.