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

On 2025-01-22 19:54 +0900, KO Myung-Hun wrote:
> Alexander Strasser wrote:
> > On 2025-01-17 22:07 +0900, KO Myung-Hun wrote:
> >> Alexander Strasser 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...
> >>>
> >>
> >> Then how about this patch? It's more simple.
> >
> > Fine for me if it is fine for you :)
> >
> > In general the GCC pointer cast warnings are legit.
> > If you disable them you will not see them when developing.
> >
> > It would still cause breakage (detected sooner or later) if
> > the code is common between OS/2 and some other platform, but
> > for OS/2 specific code new pointer cast warnings would go
> > unnoticed.
> >
> > I think I prefer your previous solution, because it is a
> > bit better in guarding breakage.
> >
>
> '-Wno-error=' does not disable warnings at all, instead it does not
> treat a warning as an error. That is, this patch makes gcc behave as
> before GCC14. The following is the output while compiling with this patch:
>
> -----
> sub/subreader.c: In function 'subcp_recode':
> sub/subreader.c:1333:34: warning: passing argument 2 of 'iconv' from
> incompatible pointer type [-Wincompatible-pointer-types]
>  1333 |                 if (iconv(icdsc, &ip, &ileft,
>       |                                  ^~~
>       |                                  |
>       |                                  char **
> In file included from sub/subreader.c:55:
> g:/usr/include/iconv.h:37:31: note: expected 'const char **' but
> argument is of type 'char **'
>    37 | extern size_t iconv (iconv_t, const char **, size_t *, char **,
> size_t *);
>       |                               ^~~~~~~~~~~~~
> -----
>
> I think you thought the '-Wno-xxx' form which disabled warnings.
>
> > You are the maintainer of MPlayer on OS/2, you have the final say.
>
> Ok, I'll apply this patch soon.

Fine for me :)

I actually misread it a bit I think. Pardon me.


Best regards and thanks,
  Alexander