Why i get segmentation fault in the following code ?

Ahti

My Code:

void EyeCare::on_startcaringButton_clicked()
{
    QErrorMessage *errorMessage = new QErrorMessage ;

    errorMessage->showMessage(GetLastErrorString ());
}

QString EyeCare::GetLastErrorString()
{
  DWORD error = GetLastError();
  if (error)
  {
    LPTSTR errorMessage;
    FormatMessage(
                FORMAT_MESSAGE_FROM_SYSTEM |
                FORMAT_MESSAGE_IGNORE_INSERTS |
                FORMAT_MESSAGE_ARGUMENT_ARRAY |
                FORMAT_MESSAGE_ALLOCATE_BUFFER,NULL,
                error,0,(LPWSTR) &errorMessage,0, NULL );
    return QString :: fromWCharArray(errorMessage) ;
  }
}

kshegunov

You should post the stack trace from the crash.

VRonin

is errorMessage null or invalid when you pass it to fromWCharArray?

LPTSTR errorMessage =NULL;
    FormatMessage(
                FORMAT_MESSAGE_FROM_SYSTEM |
                FORMAT_MESSAGE_IGNORE_INSERTS |
                FORMAT_MESSAGE_ARGUMENT_ARRAY |
                FORMAT_MESSAGE_ALLOCATE_BUFFER,NULL,
                error,0,(LPWSTR) &errorMessage,0, NULL );
Q_ASSERT(errorMessage);
    return QString :: fromWCharArray(errorMessage) ;

Ahti

@VRonin still displaying segmentation fault

see the error yourself

mrjj

Hi
running your GetLastErrorString()
in win 10, mingw compiler, Qt5.7, it does not crash.

Ahti

@mrjj i have windows 7 32bit Qt5.7 MinGW
did you checked it ?? and GetLastErrorString is not an inbuilt function i created on my own for the purpose of getting the details of the error message with the help of error code returned by GetLastError ( which infact an inbuilt function ).

mrjj

@Ahti
if you call GetLastErrorString directly, does it still crash?
update: yes i ran it.

Ahti

@mrjj yes it does work if i run it separately but not as QErrorMessage argument why ??? :0 maybe there is a problem in conversion.

Rondog

Not sure if this is a problem but the variable 'errorMessage' is type LPTSTR but cast to LPWSTR in the function FormatMessage(...).

If 'DWORD error' is null then nothing is returned from this function; it should return a QString in any case. Not returning an expected return type will probably crash the program (and should generate a warning message during compilation).

Paul Colby

@Rondog said:

the variable 'errorMessage' is type LPTSTR but cast to LPWSTR in the function FormatMessage(...)

The cast is necessary, because he's using FORMAT_MESSAGE_ALLOCATE_BUFFER, in which case the lpBuffer parameter (errorMessage in this case) is interpreted not as an LP..., but a pointer to an LP... (ie that flags causes the function to interpret the parameter as something other than what the function signature says... love win32).

Having said that, the cast is wrong (probably benign though)... it should be a cast to LPTSTR, not LPWSTR - but whether that matters or not depends on whether the Microsoft headers are set to Unicode verses non-Unicode mode. Assuming the compiler isn't complaining about the cast, and well as the use of QString::fromWCharArray without a cast, he's probably using Unicode mode, in which case the Microsoft headers define LPTSTR as LPWSTR anyway, but still not good practice.

So, just adding what's already been said:

EyeCare::GetLastErrorString should always return a value (not sure that would cause a crash though - I would have thought a default-constructed QString would be returned instead, but always better to be explicit);
you should check the result of the FormatMessage (if its 0, then errorMessage is likely to be an invalid random dangling pointer);
you should definitely initialise errorMessage to NULL, otherwise there's no way to distinguish between FormatMessage failing pre-allocating versus failing post-allocating.
(for later) you should be creating the return QString, then freeing errorMessage, then returning; otherwise you have a memory leak.

I'd do something like:

QString EyeCare::GetLastErrorString()
{
  QString message = QString::fromLatin1("no error"); // Or empty string if you prefer.
  const DWORD error = GetLastError();
  if (error != ERROR_SUCCESS) // I just like to be explicit.
  {
    // Fetch the error message.
    LPTSTR errorMessage = NULL;
    const DWORD size = FormatMessage(
                FORMAT_MESSAGE_FROM_SYSTEM |
                FORMAT_MESSAGE_IGNORE_INSERTS |
                FORMAT_MESSAGE_ARGUMENT_ARRAY |
                FORMAT_MESSAGE_ALLOCATE_BUFFER,NULL,
                error,0,(LPTSTR) &errorMessage,0, NULL );

    // Copy the error message to a QString object.
    if (size > 0) {
        #ifdef UNICODE
         message = QString::fromWCharArray(errorMessage, size);
        #else
         message = QString::fromLocal8Bit(errorMessage, size);
        #endif
    }

    // Free the error message buffer.
    if (errorMessage != NULL) {
      HeapFree(GetProcessHeap(), errorMessage);
    }
  }
  return message;
}

Cheers.

kuzulis

Just use qt_error_string() function instead.

Ahti

@Paul-Colby

Getting the following error now: :

Rondog

I don't know if it is just me but I can't see either of the posted error messages. With only a comment that says "I'm getting this now..." and without any supporting information it is tough to guess what might be happening.

mrjj

its an image it seems
0_1472284308498_sending.JPG
(/uploads/files/1472284318241-sending-resized.jpg)
Might be forum bug. An update is coming afaik, so lets see.