Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Success message when connecting to tiny trash database file [CORE2484] #2897

Closed
firebird-automations opened this issue May 30, 2009 · 9 comments

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: Bill Oliver (verbguy)

Assigned to: Claudio Valderrama C. (robocop)

Relate to CORE6518

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\);

Commits: e1c700d

@firebird-automations
Copy link
Collaborator Author

Commented by: Bill Oliver (verbguy)

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.

@firebird-automations
Copy link
Collaborator Author

Modified by: @dyemanov

Fix Version: 3.0 Alpha 1 [ 10331 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: Claudio Valderrama C. (robocop)

assignee: Claudio Valderrama C. [ robocop ]

@firebird-automations
Copy link
Collaborator Author

Commented by: Claudio Valderrama C. (robocop)

Bill, I hope you will find my code shorter.

@firebird-automations
Copy link
Collaborator Author

Modified by: Claudio Valderrama C. (robocop)

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @pcisar

status: Resolved [ 5 ] => Closed [ 6 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

QA Status: No test

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

status: Closed [ 6 ] => Closed [ 6 ]

QA Status: No test => Done successfully

@firebird-automations
Copy link
Collaborator Author

Modified by: @AlexPeshkoff

Link: This issue relate to CORE6518 [ CORE6518 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment