New API call for path conversion

[Eric Blake]

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