Return value of Function 'SendEmail' might be undefined

[Natcree]

Habe ne Fehlermeldung krieg Sie nicht weg.

Return Value of Function 'SendEmail' might be undefined.

Hiermal der Code

Delphi-Quellcode:

function SendEMail(Handle: THandle; Mail: TStrings): Cardinal;
type
  TAttachAccessArray = array [0..0] of TMapiFileDesc;
  PAttachAccessArray = ^TAttachAccessArray;
var
  MapiMessage: TMapiMessage;
  Receip: TMapiRecipDesc;
  Attachments: PAttachAccessArray;
  AttachCount: Integer;
  i1: integer;
  FileName: string;
  dwRet: Cardinal;
  MAPI_Session: Cardinal;
  WndList: Pointer;
begin
  dwRet := MapiLogon(Handle,
    PChar(''),
    PChar(''),
    MAPI_LOGON_UI or MAPI_NEW_SESSION,
    0, @MAPI_Session);

  if (dwRet <> SUCCESS_SUCCESS) then
  begin
    MessageBox(Handle,
      PChar('Error while trying to send email'),
      PChar('Error'),
      MB_ICONERROR or MB_OK);
  end
  else
  begin
    FillChar(MapiMessage, SizeOf(MapiMessage), #0);
    Attachments := nil;
    FillChar(Receip, SizeOf(Receip), #0);

    if Mail.Values['to'] <> '' then
    begin
      Receip.ulReserved := 0;
      Receip.ulRecipClass := MAPI_TO;
      Receip.lpszName := StrNew(PChar(Mail.Values['to']));
      Receip.lpszAddress := StrNew(PChar('SMTP:' + Mail.Values['to']));
      Receip.ulEIDSize := 0;
      MapiMessage.nRecipCount := 1;
      MapiMessage.lpRecips := @Receip;
    end;

    AttachCount := 0;

    for i1 := 0 to MaxInt do
    begin
      if Mail.Values['attachment' + IntToStr(i1)] = '' then
        break;
      Inc(AttachCount);
    end;

    if AttachCount > 0 then
    begin
      GetMem(Attachments, SizeOf(TMapiFileDesc) * AttachCount);

      for i1 := 0 to AttachCount - 1 do
      begin
        FileName := Mail.Values['attachment' + IntToStr(i1)];
        Attachments[i1].ulReserved := 0;
        Attachments[i1].flFlags := 0;
        Attachments[i1].nPosition := ULONG($FFFFFFFF);
        Attachments[i1].lpszPathName := StrNew(PChar(FileName));
        Attachments[i1].lpszFileName :=
          StrNew(PChar(ExtractFileName(FileName)));
        Attachments[i1].lpFileType := nil;
      end;
      MapiMessage.nFileCount := AttachCount;
      MapiMessage.lpFiles := @Attachments^;
    end;

    if Mail.Values['subject'] <> '' then
      MapiMessage.lpszSubject := StrNew(PChar(Mail.Values['subject']));
    if Mail.Values['body'] <> '' then
      MapiMessage.lpszNoteText := StrNew(PChar(Mail.Values['body']));

    WndList := DisableTaskWindows(0);
    try
    Result := MapiSendMail(MAPI_Session, Handle,
      MapiMessage, MAPI_DIALOG, 0);
    finally
      EnableTaskWindows( WndList );
    end;

    for i1 := 0 to AttachCount - 1 do
    begin
      StrDispose(Attachments[i1].lpszPathName);
      StrDispose(Attachments[i1].lpszFileName);
    end;

    if Assigned(MapiMessage.lpszSubject) then
      StrDispose(MapiMessage.lpszSubject);
    if Assigned(MapiMessage.lpszNoteText) then
      StrDispose(MapiMessage.lpszNoteText);
    if Assigned(Receip.lpszAddress) then
      StrDispose(Receip.lpszAddress);
    if Assigned(Receip.lpszName) then
      StrDispose(Receip.lpszName);
    MapiLogOff(MAPI_Session, Handle, 0, 0);
  end;
end;// Die letzte Zeile gibt er mir an

[Aviator]

Die Meldung bedeutet, dass der Rückgabewert der Funktion möglicherweise undefiniert ist. Um den Fehler zu entfernen, könntest du zu Beginn der Funktion einfach Result := 0 schreiben. Dann ist der Rückgabewert der Funktion garantiert schon einmal 0 sofern während der weiteren Ausführung ein Fehler autritt und Result nicht neu gesetzt wird.

[Natcree]

Danke Dir Fehler ist Weg. Ich habe es auch damit schon versucht aber am ende der function.

[himitsu]

Zitat von Natcree:

Ich habe es auch damit schon versucht aber am ende der function.

Das sollte auch gehn und die Meldung weg sein.

Nur daß dann der Compiler sagen sollte, daß der Rückgabewert von MapiSendMail umsonst an Result zugewiesen wird.

Delphi-Quellcode:

if (dwRet <> SUCCESS_SUCCESS) then
  begin
    MessageBox(Handle,
      PChar('Error while trying to send email'),
      PChar('Error'),
      MB_ICONERROR or MB_OK);
  end

Eigentlich ghört das fehlende Result := ...; dort rein, denn hier fehlt die Zuweisung an Result, wärend es im Else-Zweig via MapiSendMail gesetzt wird.

Und nein, 0 ist hier ein total falsches Ergebnis, denn MapiSendMail (wie man im MSDN nachlesen kann) liefert im Erfolgsfall auch eine 0. Genauer SUCCESS_SUCCESS, welche wie S_OK und NOERROR = 0 ist.

Also wenn, dann sollte man Result dort auf MAPI_E_FAILURE oder besser noch MAPI_E_LOGIN_FAILURE setzen, da es ja einen Fehler bei MapiLogon angibt.
Aber ich würde Result da auf dwRet setzen, also den "korrekten" Fehlercode. Oder dwRet gleich weglassen und direkt Result verwenden.


Und die vielen StrDispose gehören wohl eher in ein Try-Finally, also ins Finally und das Try nach dem zweiten FillChar.

[Natcree]

Ich weiss nicht ob es so richtiger Wär

oben im Code habe ich das geändert

Delphi-Quellcode:

  Result :=  MAPI_E_LOGIN_FAILURE;
  dwRet := MapiLogon(Handle,PChar(''),PChar(''),MAPI_LOGON_UI or MAPI_NEW_SESSION,
  0, @MAPI_Session);
  if (dwRet <> SUCCESS_SUCCESS) then begin
    MessageBox(Handle,PChar('Error while trying to send email'),PChar('Error'),
    MB_ICONERROR or MB_OK);
  end
  else

und unten

Delphi-Quellcode:

  if Assigned(MapiMessage.lpszSubject) then try
  StrDispose(MapiMessage.lpszSubject); finally
  if Assigned(MapiMessage.lpszNoteText) then try
  StrDispose(MapiMessage.lpszNoteText);  finally
  if Assigned(Receip.lpszAddress) then try
  StrDispose(Receip.lpszAddress); finally
  if Assigned(Receip.lpszName) then try
  StrDispose(Receip.lpszName); finally
    MapiLogOff(MAPI_Session, Handle, 0, 0);
  end;
end;
end;
end;
end;
end;

so jetzt Hagelt es spottt

[Luckie]

So ein Fehler/Warnung schleicht sich schnell mal ein bei solchen Spagetti Code ein. Und da du selbst den "Fehler" nicht gesehen hast, ist das ein Zeichen da für, dass du selbst etwas den Überblick verloren hast.

[himitsu]

Zitat von Natcree:

so jetzt Hagelt es spottt

1 Try-Finally um alle Dispose hätte bestimmt ausgereicht.
> dort im Finally sollte es hoffentlich nicht knallen und wenn doch, dann hat es vorher den Speichermanager total zerschossen und da nun ist eh alles egal zu spät.

Da die Variablen (wo die Speicher reserviert/verwaltet werden) vorher mit nil (0) initilisiert und innerhalb des try-finally reserviert werden, können die dann "problemlos" zusammen im finlly-end freigegeben werden, da bis dahin auch alle variablen auch ordentlich initialisiert wurden

[sx2008]

Zitat von Natcree:

so jetzt Hagelt es spottt

Eben.
Was ist das denn für eine Fehlerbehandlung?

Delphi-Quellcode:

    MessageBox(Handle,
      PChar('Error while trying to send email'),
      PChar('Error'),
      MB_ICONERROR or MB_OK);

Das ist viel zu ungenau und ausserdem sollte man bei Fehler Exceptions verwenden.

Delphi-Quellcode:

// z.B. so
raise Exception.CreateFmt('can''t open MAPI to send email'#13#10'return code was %d', [dwRet])

So weiss man zumindest dass MAPI verwendet wird und man kennt den Returncode.
Optimalerweise übersetzt man die MAPI Returncodes noch in Klartext und verwendet eine eigene Exception-Klasse.
Was würde wohl passieren falls jemand diesen Code schreibt und MAPI nicht richtig eingerichtet ist?

Delphi-Quellcode:

for i := 1 to 2000 do
  SendEMail(self.Handle, mailinglist);

Man müsste 2000 Mal eine Fehlermeldung bestätigen ohne zu wissen wie oft man noch klicken muss.
Dann kann man das Programm nur noch mit dem Taskmgr abschiesen.
Mit einer Exception kommt die Fehlermeldung nur einmal und die Schleife und alles davor wird abgebrochen weil es eh sinnlos ist bei dem Fehler weiterzumachen.

[himitsu]

Zitat von sx2008:

Was ist das denn für eine Fehlerbehandlung?
Das ist viel zu ungenau und ausserdem sollte man bei Fehler Exceptions verwenden.

Im Prinzip würde man hier
- entweder die MessageBox weg machen und dwRes als Result zurückliefern, denn der fehlervor vom MapiSendMail wird auch nicht geprüft und nur zurückgegeben
=> Fehlerbehandlung dann beim Aufrufer, indem er das Result von SendEMail prüft

- oder eine Exception statt der MessageBox, aber dann auch sofort das Ergebnis von MapiSendMail prüfen und ebenfalls eine exception werfen.
=> hier kommt das Result dann weg und es wird eine Prozedur daraus