Opened 16 years ago

Closed 16 years ago

#1429 closed defect (fixed)

VSI*L and "errno" setting.

Reported by: warmerdam Owned by: Mateusz Łoskot
Priority: normal Milestone: 1.5.0
Component: default Version: 1.4.0
Severity: normal Keywords: errno vsi
Cc:

Description (last modified by warmerdam)

In order to enable higher level code to report errors from the various VSI*L functions such as VSIFOpenL() it is necessary that "errno" get set. This does not currently occur on win32 (with cpl_vsil_win32.cpp code).

Minimally the Open() method in cpl_vsil_win32.cpp should try to translate win32 error codes to corresponding errno values. Maximally error setting should be done consistently through cpl_vsil_win32.cpp and cpl_vsi_mem.cpp along with some effort to test these in the test suite.

Change History (9)

comment:1 by Mateusz Łoskot, 16 years ago

Frank,
Do you think it will be enough to just map error codes, from Windows API calls and GetLastError() to POSIX codes?
The mapping means to try to match win32 code to POSIX as best as possible.
Or do you also want to support error messages corresponding to Win32 error codes?

I'm thinking about adding a free local function to the cpl_vsil_win32.cpp, ErrnoFromGetLastError() or similar, with a big switch mapping GetLastError() codes to POSIX codes.

This function could be used not only in VSIWin32FilesystemHandler::Open() but also in other VSI functions for Win32:

errno = ErrnoFromGetLastError();

What do you think?

comment:2 by warmerdam, 16 years ago

Yes, this sounds good. 

Note that there are only a few errno values the code ever checks for, so there
is no need to be comprehensive in the mapping.  Try to capture some of the
more obvious errors (permissions, file does not exist, too large). 

comment:3 by Mateusz Łoskot, 16 years ago

The bug has been fixed in r10986:
http://trac.osgeo.org/gdal/changeset/10986

Frank,

Please, verify the solution.

(BTW, I've forgot to add the bug number in the SVN log message)

comment:4 by warmerdam, 16 years ago

Could you add error translation in VSIWin32Handle::Read(), and Write() as well?

Also, could you review the functions like stat(), unlink(), rename(), mkdir()
and rmdir() to verify they set errno on failure?  

Also, please review cpl_vsi_mem.cpp and try to set errno to reasonable values.
If this is too ambiguous let me know and I'll take a crack at it. 


comment:5 by Mateusz Łoskot, 16 years ago

(In reply to comment #4)
> Could you add error translation in VSIWin32Handle::Read(),
> and Write() as well?

Done.
http://trac.osgeo.org/gdal/changeset/10992

> Also, could you review the functions like stat(), unlink(), rename(), mkdir()
> and rmdir() to verify they set errno on failure?

I'm not sure I understand it well.
These functions (in VSI WIn32) come from C library, so the errno is set by these functions already.

> Also, please review cpl_vsi_mem.cpp and try to set errno to reasonable values.
> If this is too ambiguous let me know and I'll take a crack at it. 

I walked through the cpl_vsi_mem.cpp and I don't see any places where I could add setting new errno values.
I don't know how to detect in VSI MEM errors like EACCES, EBUSY or ENOTDIR.
According to my understanding, the VSI MEM can mainly test for ENOENT, if there is no descriptor available, but this error is already supported.

comment:6 by Mateusz Łoskot, 16 years ago

What about VSIMemFilesystemHandler::ReadDir() ?
I think it's reasonable to set errno=ENOENT there.

comment:8 by warmerdam, 16 years ago

Description: modified (diff)
Milestone: 1.5.0

comment:9 by warmerdam, 16 years ago

ok, ENOENT sounds ok.

Really, the memory subsystem needs proper directory handling, but not immediately.

comment:10 by Mateusz Łoskot, 16 years ago

Keywords: errno vsi added
Resolution: fixed
Status: assignedclosed

I added ENOENT setting in r11104 and I'm resolving this bug as fixed.

Note: See TracTickets for help on using tickets.