Message-ID: <4D438432.3070906@cwilson.fastmail.fm>
Date: Fri, 28 Jan 2011 22:06:26 -0500
From: Charles Wilson <cygwin AT cwilson DOT fastmail DOT fm>
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.23) Gecko/20090812 Thunderbird/2.0.0.23 Mnenhy/0.7.6.666
MIME-Version: 1.0
CC: cygwin AT cygwin DOT com, bug-gnu-libiconv AT gnu DOT org
Subject: Re: Bug in libiconv?
References: <201101282312 DOT 50298 DOT bruno AT clisp DOT org>
In-Reply-To: <201101282312.50298.bruno@clisp.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com

On 1/28/2011 5:12 PM, Bruno Haible wrote:
>> the old cygwin_conv_to_posix_path function as well.
> 
> Is cygwin_conv_to_posix_path deprecated? Does it introduce limitations of
> some kind?

Yes, and (and because:) yes.

The limitation is, the old functions:

extern int cygwin_win32_to_posix_path_list (const char *, char *)
extern int cygwin_win32_to_posix_path_list_buf_size (const char *)
extern int cygwin_posix_to_win32_path_list (const char *, char *)
extern int cygwin_posix_to_win32_path_list_buf_size (const char *)
extern int cygwin_conv_to_win32_path (const char *, char *)
extern int cygwin_conv_to_full_win32_path (const char *, char *)
extern int cygwin_conv_to_posix_path (const char *, char *)
extern int cygwin_conv_to_full_posix_path (const char *, char *)

are all deprecated, because (a) they don't handle wide chars, (b) and
are limited to only 254 char path lengths.  The replacement functions

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

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

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

do not have these limitations (well, 4Kbytes/2k wchars for a single
filename; 32K? for pathlists).  cygwin_conv_path_t controls the
behavior, and can accept the following values:

enum
{
  CCP_POSIX_TO_WIN_A = 0, /* from is char*, to is char*       */
  CCP_POSIX_TO_WIN_W,     /* from is char*, to is wchar_t*    */
  CCP_WIN_A_TO_POSIX,     /* from is char*, to is char*       */
  CCP_WIN_W_TO_POSIX,     /* from is wchar_t*, to is char*    */

  /* Or these values to the above as needed. */
  CCP_ABSOLUTE = 0,       /* Request absolute path (default). */
  CCP_RELATIVE = 0x100    /* Request to keep path relative.   */
};

However, by using the linux-ish facilities throughout and avoiding the
win32 stuff, you can ALSO avoid the necessity of calling any path
conversion functions at all -- and eliminate a lot of platform-specific
code.  (e.g. let the cygwin dll do ALL the work)

>> The usage of a fixed table instaed of the charset.alias file in
>> libcharset/lib/localcharset.c, function get_charset_aliases() is
>> not good, not good at all.
> 
> The alternative is to have this table stored in a file charset.alias;
> but then every package that includes the module 'localcharset' from
> gnulib (that is, libiconv, gettext, coreutils, and many others) will
> want to modify this file during "make install". And this causes a lot of
> headaches to packaging systems. Therefore, on platforms which have
> widely used packaging systems (Linux,

huh?

> MacOS X, Cygwin), it's better to
> avoid the need for this file.

From inspecting the code, it sure looks like linux still uses the
charset.alias file to me, at least in the released version of
libiconv-1.13.1 (of course, most actual linux platforms don't install
libiconv anyway, since glibc handles that).

> Additionally, on Win32 systems relocatability
> is a must, and the code to compute the location of charset.alias from
> the location of libiconv.dll would be overkill.

Meh, for win32.  Cygwin -- not so much, since cygwin handles the
"relocation" itself, relative to the underlying win32 paths, via its
mount point emulation.  cygiconv-2.dll is always in the
cygwin-translated path "/usr/bin", and charset.alias is in "/usr/lib".

> The reason for these "&& !defined __CYGWIN__" clauses is that - at least
> in Cygwin 1.5.x - gcc has an option that will define _WIN32 or __WIN32__.

Yes, or if you #include <windows.h>.  I was talking about the fact that
in several places, libiconv explicitly #includes windows.h -- and that
triggers _WIN32 and __WIN32__ to be defined, regardless of any gcc
command line args.  Avoid including windows.h, and...

> So, when _WIN32 || __WIN32__ may evaluate to true on Cygwin, or it may
> evaluate to false on Cygwin. Since I don't want libiconv or gettext
> to be compiled in two possible ways on Cygwin, I add
> "&& !defined __CYGWIN__".

Except now we (might) need to distinguish between OLD cygwin (1.5) and
current, supported cygwin (1.7+).  Since __STDC_ISO_10646__ is defined
by the latter (at least, 1.7.8+, even if the underlying functionality
works back to 1.7.2) -- but is NOT defined on cygwin-1.5...it seems ok
to use that symbol to "distinguish" between the two flavors -- except
that this removes your "safety net" concerning __WIN32__ getting defined
on Cygwin, for old cygwin where __STDC_ISO_10646__ is not defined.

> Neither libiconv nor gettext defines or undefines _WIN32 or __WIN32__.
> But they are prepared to either setting.

Should libiconv support for cygwin be 1.7 only, or 1.5 only?  Supporting
both is going seriously uglify the code.  And 1.5 is no longer supported
even by cygwin.com.

You *could* do this:
AC_CHECK_DECLS([cygwin_conv_path], [],[], [[#include <sys/cygwin.h>]])

and use __CYGWIN__ && HAVE_DECL_CYGWIN_CONV_PATH to mean "new" cygwin,
and __CYGWIN__ && !HAVE_DECL_CYGWIN_CONV_PATH to mean "old" cygwin,
but...ugly, horrid, icky...

>>  - 'iconv_close ((iconv_t) -1);' crashes the application with a SEGV.
> 
> It's not a bug. From POSIX:2008
...
> "may", not "shall".

Well, sure.  But it /would/ be nice to just set errno (EINVAL?) instead
of segfaulting...

--
Chuck

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple