Issue Details (XML | Word | Printable)

Key: CORE-2484
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Trivial Trivial
Assignee: Claudio Valderrama C.
Reporter: Bill Oliver
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Firebird Core

Success message when connecting to tiny trash database file

Created: 29/May/09 09:41 PM   Updated: 15/Feb/16 04:15 AM
Component/s: Engine
Affects Version/s: 1.5.5, 2.0.4, 2.1.1, 2.5 Beta 1
Fix Version/s: 3.0 Alpha 1

Environment: windows only

QA Status: Done successfully


 Description  « Hide
Create a trash database - for example:
   echo Bogus! > trash.fdb

Now try to connect to it:

C:\firebird2_head\firebird2win1\temp\Win32\Debug\firebird\bin>isql -u sysdba -p masterkey trash.fdb
Statement failed, SQLCODE = 08001
I/O error for file "C:\FIREBIRD2_HEAD\FIREBIRD2WIN1\TEMP\WIN32\DEBUG\FIREBIRD\BIN\TRASH.FDB"
-Error while trying to read from file
-The operation completed successfully.

Note error message "the operation completed successfully." That is incorrect, and should not be present in status vector.

.....

The problem is occurring in jrd/os/win32/winnt.cpp.

We have one error routine, nt_error(), that is called in case of windows file system errors.

In the test case at hand, we call Windows routine ReadFile(). This routine returns 0 if the read is unsuccessful. Additionally, we have function parameter lpNumberOfBytesRead, which can be used to tell how many bytes are read.

In this test is that the read is successful, but we go to the error handler, b/c the number of bytes read doesn't match the expected value. So, we add ERROR_SUCCESS to the status vector.

The simple fix is to assign the value of GetLastError() to a local variable and if it is equal to ERROR_SUCCESS, then don't add it to the status vector.

Here is a trivial patch for the problem, certainly the final patch may be improved upon.

With this patch, the windows success message is suppressed, but we still get the "error reading from file" message.

C:\firebird2_head\firebird2win1\temp\Win32\Debug\firebird\bin>isql -u sysdba -p masterkey trash.fdb
Statement failed, SQLCODE = 08001
I/O error for file "C:\FIREBIRD2_HEAD\FIREBIRD2WIN1\TEMP\WIN32\DEBUG\FIREBIRD\BIN\TRASH.FDB"
-Error while trying to read from file
Use CONNECT or CREATE DATABASE to specify a database
SQL>

...

### Eclipse Workspace Patch 1.0
#P firebird2win1
Index: src/jrd/os/win32/winnt.cpp
===================================================================
RCS file: /cvsroot/firebird/firebird2/src/jrd/os/win32/winnt.cpp,v
retrieving revision 1.83
diff -u -r1.83 winnt.cpp
--- src/jrd/os/win32/winnt.cpp 28 Apr 2009 13:07:49 -0000 1.83
+++ src/jrd/os/win32/winnt.cpp 29 May 2009 19:03:25 -0000
@@ -1102,15 +1102,27 @@
  * to do something about it. Harumph!
  *
  **************************************/
+ DWORD lastError = GetLastError();
+
  if (!status_vector)
  {
- ERR_post(Arg::Gds(isc_io_error) << Arg::Str(string) << Arg::Str(file->fil_string) <<
- Arg::Gds(operation) << Arg::Windows(GetLastError()));
+ if (lastError != ERROR_SUCCESS)
+ ERR_post(Arg::Gds(isc_io_error) << Arg::Str(string) << Arg::Str(file->fil_string) <<
+ Arg::Gds(operation) << Arg::Windows(GetLastError()));
+ else
+ ERR_post(Arg::Gds(isc_io_error) << Arg::Str(string) << Arg::Str(file->fil_string) <<
+ Arg::Gds(operation));
+
  }
 
- ERR_build_status(status_vector,
- Arg::Gds(isc_io_error) << Arg::Str(string) << Arg::Str(file->fil_string) <<
- Arg::Gds(operation) << Arg::Windows(GetLastError()));
+ if (lastError != ERROR_SUCCESS)
+ ERR_build_status(status_vector,
+ Arg::Gds(isc_io_error) << Arg::Str(string) << Arg::Str(file->fil_string) <<
+ Arg::Gds(operation) << Arg::Windows(GetLastError()));
+ else
+ ERR_build_status(status_vector,
+ Arg::Gds(isc_io_error) << Arg::Str(string) << Arg::Str(file->fil_string) <<
+ Arg::Gds(operation));
 
  gds__log_status(0, status_vector);
 



 All   Comments   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Bill Oliver added a comment - 29/May/09 09:48 PM
The final commit should use the local variable lastError in the error routines, instead of calling GetLastError() again. For example,

ERR_post(Arg::Gds(isc_io_error) << Arg::Str(string) << Arg::Str(file->fil_string) <<
   Arg::Gds(operation) << Arg::Windows(lastError));

Sorry for extra noise.

Claudio Valderrama C. added a comment - 30/Nov/09 06:06 AM
Bill, I hope you will find my code shorter.