Re: Cast "const char *" pointers to "char *" to avoid compiler warnings.

On Wed, 10 Oct 2018 at 17:02, Corinna Vinschen <[email protected]> wrote:
>
> On Oct 10 16:37, Christophe Lyon wrote:
> > On Wed, 10 Oct 2018 at 11:22, Corinna Vinschen <[email protected]> wrote:
> > >
> > > On Oct  2 11:10, Christophe Lyon wrote:
> > > > On Tue, 2 Oct 2018 at 00:50, Craig Howland <[email protected]> wrote:
> > > > >
> > > > > On 10/01/2018 05:33 PM, Christophe Lyon wrote:
> > > > > > Hi,
> > > > > >
> > > > > > GCC complains that some assignments loose the const-ness of several
> > > > > > data. This small patch adds explicit (char *) casts, but I'm not
> > > > > > familiar enough with what newlib does with these to be sure that they
> > > > > > are not modified. Maybe the proper fix would be to declare the
> > > > > > destinations as "const"?
> > > > > >
> > > > > > Christophe
> > > > > If I understand what you're saying properly, it amounts to saying that you did
> > > > > not verify whether the GCC warnings about discarding const are valid or not, yet
> > > > > you are suppressing them.  Is this a proper understanding?  If so, it seems like
> > > > > these proposed patches are a bad idea, as they might be hiding a real problem,
> > > > > or changing the wrong thing.  (In a very quick look at locale.c, for example,
> > > > > locale can definitely be written to--it is definitely not const. This implies
> > > > > that the const on new_locale is what is wrong.)
> > > >
> > > > I did have this "very quick look at locale.c" before writing the
> > > > patch, and not adding
> > > > the cast at the assignment point means removing "const" from __loadlocale()
> > > > prototype:
> > > > char *__loadlocale (struct __locale_t *loc, int category, const char
> > > > *new_locale)
> > > > which in turn has a significant impact on the callers which I hope
> > > > people familiar
> > > > with this area can confirm, or not.
> > >
> > > The bug was, in fact, to define the third parameter as const, given it
> > > gets potentially overwritten.  None of the incoming values is const
> > > anyway.  I pushed a patch.
> > >
> >
> > Thanks.
> >
> > However, when building for aarch64, I'm still seeing:
> > newlib/libc/ctype/ctype_.c:179:16: warning: assignment discards
> > ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
> >
> > And I think my patch (or something similar) is still needed for jp2uc.c ?
>
> I only had a look into the __loadlocal issue due to this discussion.
> For everything else, please send a new patch.
>

OK, here is the patch for jp2uc.c:

>
> Corinna
>
> --
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat

commit 5d108977d84d506c0020b075775ed49f8d14a89b
Author: Christophe Lyon <[email protected]>
Date:   Fri Oct 5 09:11:05 2018 +0000

    Declare "cs" variable as "const char *"
    
    Instead of "char *" to avoid compiler warnings.
    This is OK because "cs" is only used as input of strcmp.
    
    2018-10-01  Christophe Lyon  <[email protected]>
    
    	* newlib/libc/ctype/jp2uc.c (_jp2uc_l, _uc2jp_l): Declare "cs" as
    	const.

diff --git a/newlib/libc/ctype/jp2uc.c b/newlib/libc/ctype/jp2uc.c
index b89b5ea..5e30f09 100644
--- a/newlib/libc/ctype/jp2uc.c
+++ b/newlib/libc/ctype/jp2uc.c
@@ -166,7 +166,7 @@ __uc2jp (wint_t c, int type)
 wint_t
 _jp2uc_l (wint_t c, struct __locale_t * l)
 {
-  char * cs = l ? __locale_charset(l) : __current_locale_charset();
+  const char * cs = l ? __locale_charset(l) : __current_locale_charset();
   if (0 == strcmp (cs, "JIS"))
     c = __jp2uc (c, JP_JIS);
   else if (0 == strcmp (cs, "SJIS"))
@@ -186,7 +186,7 @@ _jp2uc (wint_t c)
 wint_t
 _uc2jp_l (wint_t c, struct __locale_t * l)
 {
-  char * cs = l ? __locale_charset(l) : __current_locale_charset();
+  const char * cs = l ? __locale_charset(l) : __current_locale_charset();
   if (0 == strcmp (cs, "JIS"))
     c = __uc2jp (c, JP_JIS);
   else if (0 == strcmp (cs, "SJIS"))