Opened 14 years ago

Closed 14 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 Changed 14 years ago by Mateusz Łoskot

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 Changed 14 years ago by warmerdam

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 Changed 14 years ago by Mateusz Łoskot

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 Changed 14 years ago by warmerdam

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 Changed 14 years ago by Mateusz Łoskot

(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 Changed 14 years ago by Mateusz Łoskot

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

comment:8 Changed 14 years ago by warmerdam

Description: modified (diff)
Milestone: 1.5.0

comment:9 Changed 14 years ago by warmerdam

ok, ENOENT sounds ok.

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

comment:10 Changed 14 years ago by Mateusz Łoskot

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.