Re: [PATCH] Fix compilation errors due to a wrong declaration of iconv() on OS/2

On 14 January 2025 22:51:12 CET, Alexander Strasser <[email protected]> wrote:
>Hi/2 KO Myung-Hun :)
>
>On 2025-01-09 14:36 +0900, KO Myung-Hun wrote:
>> Recent compilers such as GCC 14 generate errors for implicit casts.
>> ---
>>  configure             |  3 ++-
>>  osdep/include/iconv.h | 32 ++++++++++++++++++++++++++++++++
>>  2 files changed, 34 insertions(+), 1 deletion(-)
>>  create mode 100644 osdep/include/iconv.h
>>
>> diff --git a/configure b/configure
>> index 753e14446..e6634bba2 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1961,7 +1961,8 @@ if os2 ; then
>>    test -n "$ld_static" && ld_static='-Wl,-static'
>>    # OS/2 linker does not support a thin archive. Remove 'T' flag.
>>    _arflags=$(echo $_arflags | tr -d T)
>> -  extra_cflags="$extra_cflags -D_EMX_SOURCE"
>> +  # -Iosdep/include is for MPlayer, -I../osdep/include is for FFmpeg
>> +  extra_cflags="$extra_cflags -D_EMX_SOURCE -Iosdep/include -I../osdep/include"
>>  fi
>
>It's a bit unusual compared to the normal use of headers in osdep/ .
>It should probably work OK though.
>
>Maybe we should actually change it in the rest of the code base to
>work this way, but that would require more careful planning, because
>than it's no more possible to select the usage per os, like you do
>here, which is also a bit different.
>
>Or at some point we switch it to the FFmpeg compat scheme. Dunno...
>
>
>>  if wine ; then
>> diff --git a/osdep/include/iconv.h b/osdep/include/iconv.h
>> new file mode 100644
>> index 000000000..daf6d1108
>> --- /dev/null
>> +++ b/osdep/include/iconv.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * iconv.h for platforms such as OS/2 with non-POSIX compliant iconv.h
>> + *
>> + * Copyright (c) 2025 KO Myung-Hun ([email protected])
>> + *
>> + * This file is part of MPlayer.
>> + *
>> + * MPlayer is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * MPlayer is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with MPlayer; if not, write to the Free Software Foundation, Inc.,
>> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>> + */
>> +
>> +#ifndef MPLAYER_ICONV_H
>> +#define MPLAYER_ICONV_H
>> +
>> +#define iconv iconv_not_posix
>> +#include_next <iconv.h>
>> +
>> +#undef iconv
>> +size_t iconv(iconv_t, char **, size_t *, char **, size_t *);
>> +
>> +#endif
>> --
>
>This is similar to the tricks done in FFmpeg compat.
>
>For now I'm OK with this patch for OS/2 even though it is not in line
>with typical osdep usage.
>
>Maybe wait a few more days, if someone else wants to comment.

Seems fine to me, even if it's not exactly beautiful...