This is the mail archive of the cygwin-developers mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: New API call for path conversion


According to Corinna Vinschen on 2/22/2008 5:19 AM:
I'm mulling over a new API call for the path conversion between POSIX
and Win32 paths.  The old API (cygwin_conv_to_full_win32_path, etc) is
just not feasible anymore.  They don't allow to request the required
buffer lengths except for the path_list functions.  They don't allow to
give the buffer size as parameter but simply assume they are big enough,
which is quite dangerous.  They don't allow to specify Win32 paths in
WCHAR.

Agreed wholeheartedly. Any interface with an implicit maximum buffer length is asking for problems.



typedef enum { CCP_WIN_A_TO_POSIX, /* from is char*, to is char* */ CCP_WIN_W_TO_POSIX, /* from is wchar_t*, to is char* */ CCP_POSIX_TO_WIN_A, /* from is char*, to is char* */ CCP_POSIX_TO_WIN_W /* from is char*, to is wchar_t* */ } cygwin_conv_path_t;

  ssize_t cygwin_conv_path (cygwin_conv_path_t what, const void *from,
                            void *to, size_t size);

I like the name, and the idea of an enum for making a single entry point more flexible than a multitude of converters. Shouldn't you use restrict on the two pointers, to guarantee non-overlap between the buffers and for potentially better code generation? (That opens a different can of worms - cygwin could certainly use restrict in a lot more places).



"size" is the size of the "to" buffer in bytes (not in characters).


If size is 0, cygwin_conv_path returns the required buffer size in
bytes.

Including or excluding the trailing NUL? I'd argue for excluding.



Otherwise, it returns 0 on success, or -1 on error and errno is set to one of the below values:

I slightly disagree with these semantics. My first thought was that you should always return the required buffer size in bytes (think snprintf), whether or not it is larger than the input size. And if the incoming size is too small (but not 0), then guarantee that to[size-1]=='\0'. A nonnegative result means success, -1 on error (I guess that means converting "" should set errno to ENOENT and return -1, rather than returning 0?).



EINVAL what has an invalid value. EFAULT from or to point into nirvana. ENAMETOOLONG the resulting path is longer than 32K, or, in case of what == CCP_POSIX_TO_WIN_A, longer than MAX_PATH.

Now that MAX_PATH is 4k, don't you mean longer than Window's pathetic 256 byte limit?


ENOSPC size is less than required for the conversion.

Hmm - based on my above argument (snprintf-like semantics of always returning the necessary size and a truncated conversion), you'd never fail with ENOSPC. On the other hand, a truncated path is potentially dangerous, as it can lead to operations on the wrong file if the code did not check for the result being larger than the input size. So your idea of returning -1 and ENOSPC on insufficient non-zero buffer size is better after all. And maybe for safety, we should try to set the output buffer to "" on failure, so that if someone goofs and tries to use the converter without checking for errors, they'd at least get ENOENT or EFAULT on the subsequent operations rather silently using the original contents of the output buffer? However, I'd still like the success result to be the size of the conversion in bytes, rather than 0, since it may be smaller than the size of my input buffer, and since it avoids me having to revisit the buffer contents with strlen to find something that the converter already knew.


Maybe we should add another function which always allocates the required
buffer, along these lines:

  void *cygwin_create_path (cygwin_conv_path_t what,
                            const void *from);

Yes, that would be a nice convenience wrapper.



Return value is the pointer to the converted path or NULL with errno set as above.

or to ENOMEM if malloc failed.



The old conversion functions should continue to work. They should just
be guarded against resulting paths longer than MAX_PATH

MAX_PATH, or 256?


> and exit with an
api_fatal error if the resulting path would be longer.  This could help to
convert applications using the old API to the new one.

Does that sound reasonable?

Yes.


--
Don't work too hard, make some time for fun as well!

Eric Blake ebb9@byu.net


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]