Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Re: iconv(3) protype mismatch with POSIX

0 views
Skip to first unread message

David Holland

unread,
Jun 15, 2016, 11:28:57 PM6/15/16
to
On Sun, May 29, 2016 at 03:54:29AM +0200, Kamil Rytarowski wrote:
> On 29.05.2016 03:15, David Holland wrote:
> > On Sat, May 28, 2016 at 03:45:35PM +0200, Kamil Rytarowski wrote:
> > > [iconv const mess]
> >
> > So it appears that given
> >
> > size_t
> > iconv(iconv_t cd,
> > char **restrict src, size_t *restrict srcleft,
> > char **restrict dst, size_t *restrict dstleft);
> > size_t
> > __iconv_const(iconv_t cd,
> > const char **restrict src, size_t *restrict srcleft,
> > char **restrict dst, size_t *restrict dstleft);
> >
> > one can do
> >
> > #define iconv(cd, src, srcleft, dst, dstleft) \
> > _Generic(src, const char **: __iconv_const, default: iconv) \
> > (cd, src, srcleft, dst, dstleft)
> >
> > and at least with the gcc5 in current it seems to match as intended.
> > Plus if anything unexpected comes up #undef iconv makes the magic go
> > away.
> >
> > (Also, because implementing things as macros is not 100% benign it
> > should maybe be disabled by default in strict posix mode.)
> >
> > Anyone want to check if it works in clang?
>
> This is an interesting exercise to use C11.. however:
> 1. Not all ports moved to gcc 4.9+,
> 2. pcc doesn't support it,
> 3. it won't work as a valid and acceptable C++ code.
> 4. Many software expects system headers to be C89, GNU89 etc, and
> doesn't request C11.

None of that matters, it just needs to be wrapped in suitable ifdefs.

> How about the Solaris 11 move? Leave "extern" option for those who rally
> want it and SUS/POSIX for others.

"extern" option?

--
David A. Holland
dhol...@netbsd.org

Kamil Rytarowski

unread,
Jun 16, 2016, 6:28:39 AM6/16/16
to
On 16.06.2016 05:28, David Holland wrote:
> On Sun, May 29, 2016 at 03:54:29AM +0200, Kamil Rytarowski wrote:
> > This is an interesting exercise to use C11.. however:
> > 1. Not all ports moved to gcc 4.9+,
> > 2. pcc doesn't support it,
> > 3. it won't work as a valid and acceptable C++ code.
> > 4. Many software expects system headers to be C89, GNU89 etc, and
> > doesn't request C11.
>
> None of that matters, it just needs to be wrapped in suitable ifdefs.
>

All of them matters to me and C11 in a public-header is no-go.

Also if we allow the POSIX version in our headers then the non-complaint
one is pointless, as no modern, popular and general-purpose OS is in
"our" camp with "improved" API.

I keep paying this debt all over again in new software that never
assumes pre-2007 or so version of iconv(3) in Linux. Our patches for it
in pkgsrc are pending for years (like in qt4) nobody bothered to even
upstream "better" iconv(3).

> > How about the Solaris 11 move? Leave "extern" option for those who rally
> > want it and SUS/POSIX for others.
>
> "extern" option?
>

NAME

iconv - code conversion function

SYNOPSIS

Default

#include <iconv.h>

extern size_t iconv(iconv_t cd, const char **restrict inbuf,
size_t *restrict inbytesleft, char **restrict outbuf,
size_t *restrict outbytesleft);


SUSv3
#include <iconv.h>

size_t iconv(iconv_t cd, char **restrict inbuf,
size_t *restrict inbytesleft, char **restrict outbuf,
size_t *restrict outbytesleft);


https://illumos.org/man/3C/iconv

These calls with and without const should be ABI compatible.

Taylor R Campbell

unread,
Jun 16, 2016, 9:34:19 AM6/16/16
to
Date: Thu, 16 Jun 2016 12:27:47 +0200
From: Kamil Rytarowski <n...@gmx.com>

On 16.06.2016 05:28, David Holland wrote:
> On Sun, May 29, 2016 at 03:54:29AM +0200, Kamil Rytarowski wrote:
> > This is an interesting exercise to use C11.. however:
> > 1. Not all ports moved to gcc 4.9+,
> > 2. pcc doesn't support it,
> > 3. it won't work as a valid and acceptable C++ code.
> > 4. Many software expects system headers to be C89, GNU89 etc, and
> > doesn't request C11.
>
> None of that matters, it just needs to be wrapped in suitable ifdefs.

All of them matters to me and C11 in a public-header is no-go.

We already have plenty of conditionals like this for C99 and C++ in
public header files -- for example, __restrict, __BEGIN_DECLS, &c.
Can't hurt to add

#if (__STDC_VERSION__ - 0) > 201112L
#define iconv ...
#endif

or whatever, if we want to do that.

> > How about the Solaris 11 move? Leave "extern" option for those who rally
> > want it and SUS/POSIX for others.
>
> "extern" option?

[...]

https://illumos.org/man/3C/iconv

These calls with and without const should be ABI compatible.

Can you explain how an application uses this, and how this helps?
Does illumos use _POSIX_C_SOURCE vs _SOLARIS_SOURCE or something to
select the declaration?

David Holland

unread,
Jun 16, 2016, 2:17:08 PM6/16/16
to
On Thu, Jun 16, 2016 at 12:27:47PM +0200, Kamil Rytarowski wrote:
> > > This is an interesting exercise to use C11.. however:
> > > 1. Not all ports moved to gcc 4.9+,
> > > 2. pcc doesn't support it,
> > > 3. it won't work as a valid and acceptable C++ code.
> > > 4. Many software expects system headers to be C89, GNU89 etc, and
> > > doesn't request C11.
> >
> > None of that matters, it just needs to be wrapped in suitable ifdefs.
>
> All of them matters to me and C11 in a public-header is no-go.

Uh, I don't follow. What's wrong with

#if defined(_NETBSD_SOURCE_) && \
defined(__GNUC_PREREQ__(5,3)) && !defined(__cplusplus)
(polymorphic iconv as suggested)
#else
(posix iconv)
#endif

?

> > "extern" option?
>
> NAME
>
> iconv - code conversion function
>
> SYNOPSIS
>
> Default
>
> #include <iconv.h>
>
> extern size_t iconv(iconv_t cd, const char **restrict inbuf,
> size_t *restrict inbytesleft, char **restrict outbuf,
> size_t *restrict outbytesleft);
>
>
> SUSv3
> #include <iconv.h>
>
> size_t iconv(iconv_t cd, char **restrict inbuf,
> size_t *restrict inbytesleft, char **restrict outbuf,
> size_t *restrict outbytesleft);

Uh, I have no idea what you're getting at.

Valery Ushakov

unread,
Jun 16, 2016, 5:20:26 PM6/16/16
to
Kamil Rytarowski <n...@gmx.com> wrote:
> On 16.06.2016 05:28, David Holland wrote:
>> On Sun, May 29, 2016 at 03:54:29AM +0200, Kamil Rytarowski wrote:
>> >
>> > How about the Solaris 11 move? Leave "extern" option for those who rally
>> > want it and SUS/POSIX for others.
>>
>> "extern" option?
>
> NAME
>
> iconv - code conversion function
>
> SYNOPSIS
>
> Default
>
> #include <iconv.h>
>
> extern size_t iconv(iconv_t cd, const char **restrict inbuf,
> size_t *restrict inbytesleft, char **restrict outbuf,
> size_t *restrict outbytesleft);
>
>
> SUSv3
> #include <iconv.h>
>
> size_t iconv(iconv_t cd, char **restrict inbuf,
> size_t *restrict inbytesleft, char **restrict outbuf,
> size_t *restrict outbytesleft);
>
>
> https://illumos.org/man/3C/iconv

Uhm, color me stupid, but I don't see how "extern" makes any
difference here.

| 6.2.2 Linkages of identifiers
...
| [#5] If the declaration of an identifier for a function has
| no storage-class specifier, its linkage is determined
| exactly as if it were declared with the storage-class
| specifier extern.

I suspect the line with extern was just pasted into synopsis from the
C header (dunno about Illumos, but at on Solaris the headers do use
"extern" for iconv declaration).

Solaris header declares iconv with const for xpg5 and w/out for xpg6.

-uwe

Kamil Rytarowski

unread,
Jun 16, 2016, 5:25:27 PM6/16/16
to


On 16.06.2016 20:16, David Holland wrote:
> On Thu, Jun 16, 2016 at 12:27:47PM +0200, Kamil Rytarowski wrote:
> > > > This is an interesting exercise to use C11.. however:
> > > > 1. Not all ports moved to gcc 4.9+,
> > > > 2. pcc doesn't support it,
> > > > 3. it won't work as a valid and acceptable C++ code.
> > > > 4. Many software expects system headers to be C89, GNU89 etc, and
> > > > doesn't request C11.
> > >
> > > None of that matters, it just needs to be wrapped in suitable ifdefs.
> >
> > All of them matters to me and C11 in a public-header is no-go.
>
> Uh, I don't follow. What's wrong with
>
> #if defined(_NETBSD_SOURCE_) && \
> defined(__GNUC_PREREQ__(5,3)) && !defined(__cplusplus)
> (polymorphic iconv as suggested)
> #else
> (posix iconv)
> #endif
>
> ?
>

I understood that the default-fallback one will be the non complainant one.

Is it going to be a temporary solution?

_Generic is available since GCC-4.9, a more portable implementation is
to use __builtin_constant_p + __builtin_choose_expr... however there are
still plenty of compilers without support for similar features (like PCC).

In C++ there is a polymorphic solution out-of-the-box in C++98 with
function overloading.

> > > "extern" option?
> >
> > NAME
> >
> > iconv - code conversion function
> >
> > SYNOPSIS
> >
> > Default
> >
> > #include <iconv.h>
> >
> > extern size_t iconv(iconv_t cd, const char **restrict inbuf,
> > size_t *restrict inbytesleft, char **restrict outbuf,
> > size_t *restrict outbytesleft);
> >
> >
> > SUSv3
> > #include <iconv.h>
> >
> > size_t iconv(iconv_t cd, char **restrict inbuf,
> > size_t *restrict inbytesleft, char **restrict outbuf,
> > size_t *restrict outbytesleft);
>
> Uh, I have no idea what you're getting at.
>

I'm not familiar with opensolaris development.. I got an example:

#ifdef _XPG6
extern size_t iconv(iconv_t, char **_RESTRICT_KYWD,
size_t *_RESTRICT_KYWD, char **_RESTRICT_KYWD,
size_t *_RESTRICT_KYWD);
#else
extern size_t iconv(iconv_t, const char **_RESTRICT_KYWD,
size_t *_RESTRICT_KYWD, char **_RESTRICT_KYWD,
size_t *_RESTRICT_KYWD);
#endif

--
https://github.com/illumos/illumos-gate/blob/5a4ef21a18dfdc65328821a265582d03e85a97c9/usr/src/head/iconv.h


Assuming that the POSIX version has been already proposed as the default
one, how about:

#if defined(_NETBSD_SOURCE) && defined(_CONSTIFIED_ICONV)
size_t iconv(iconv_t, const char ** __restrict,
size_t * __restrict, char ** __restrict,
size_t * __restrict);
#else
size_t iconv(iconv_t, const char ** __restrict,
size_t * __restrict, char ** __restrict,
size_t * __restrict);
#endif

It will work for all c(89+)/c++(98+)/objc/.. compilers.

With that we could build software with additional CPPFLAGS defines when
needed.

Next step is to detect what fails to build in pkgsrc and adjust it
accordingly.

0 new messages