| Message ID | CAKdteOaix2wyhp62oQrmf6TigBOBdJK1cVDJMzFqqM2b47KHOw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Cast "const char *" pointers to "char *" to avoid compiler warnings. | expand |
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.) Craig
On Tue, 2 Oct 2018 at 00:50, Craig Howland <howland@lgsinnovations.com> 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. It looks like several of these small patches uncover inconsistencies. > Craig > > >
On Tue, Oct 2, 2018 at 5:10 AM Christophe Lyon <christophe.lyon@linaro.org> wrote: >... (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. As I understand it, you can only cast const away if the [original] object is in fact non-const. OK: void foo(const char* buff) { ((char*)buf)[0] = '\0'; } char bar[10] = {0}; foo(bar); Not OK: void foo(const char* buff) { ((char*)buf)[0] = (char)1; } const char bar[10] = {0}; foo(bar); The tricky part is finding the original object. I don't know if there is a sanitizer to help find these violations, but I have often wondered about one. Jeff
On 01/10/18 22:33, 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 > > > newlib-4.txt > > > commit 349a08c43cd4baf0d93f28ea8ca7351bf9606d50 > Author: Christophe Lyon <christophe.lyon@linaro.org> > Date: Mon Oct 1 18:53:37 2018 +0000 > > Cast "const char *" pointers to "char *" to avoid compiler warnings. > > 2018-10-01 Christophe Lyon <christophe.lyon@linaro.org> > > * newlib/libc/ctype/ctype_.c (__set_ctype): Cast "_ctype_" to "char *". > * newlib/libc/ctype/jp2uc.c (_jp2uc_l, _uc2jp_l): Cast output of > "__locale_charset()" and "__current_locale_charset()" to "char *". > * newlib/libc/locale/locale.c (__loadlocale): Cast "new_locale" to > "char *". > > diff --git a/newlib/libc/ctype/ctype_.c b/newlib/libc/ctype/ctype_.c > index 28727e8..851fc06 100644 > --- a/newlib/libc/ctype/ctype_.c > +++ b/newlib/libc/ctype/ctype_.c > @@ -176,7 +176,7 @@ __set_ctype (struct __locale_t *loc, const char *charset) > # if defined(ALLOW_NEGATIVE_CTYPE_INDEX) > ctype_ptr = _ctype_b; > # else > - ctype_ptr = _ctype_; > + ctype_ptr = (char *) _ctype_; > # endif > } > # if defined(ALLOW_NEGATIVE_CTYPE_INDEX) > diff --git a/newlib/libc/ctype/jp2uc.c b/newlib/libc/ctype/jp2uc.c > index b89b5ea..00272eb 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(); > + char * cs = l ? (char *) __locale_charset(l) : (char *) __current_locale_charset(); Why not change cs to const char *? It's only used to call strcmp. Probably most other casts should be similarly considered. Generally, this patch feels wrong, IMO. R. > 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(); > + char * cs = l ? (char *) __locale_charset(l) : (char *) __current_locale_charset(); > if (0 == strcmp (cs, "JIS")) > c = __uc2jp (c, JP_JIS); > else if (0 == strcmp (cs, "SJIS")) > diff --git a/newlib/libc/locale/locale.c b/newlib/libc/locale/locale.c > index 791a775..79da35f 100644 > --- a/newlib/libc/locale/locale.c > +++ b/newlib/libc/locale/locale.c > @@ -515,7 +515,7 @@ restart: > } > # define FAIL goto restart > #else > - locale = new_locale; > + locale = (char *) new_locale; > # define FAIL return NULL > #endif > >
On Tue, 2 Oct 2018 at 11:26, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote: > > On 01/10/18 22:33, 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 > > > > > > newlib-4.txt > > > > > > commit 349a08c43cd4baf0d93f28ea8ca7351bf9606d50 > > Author: Christophe Lyon <christophe.lyon@linaro.org> > > Date: Mon Oct 1 18:53:37 2018 +0000 > > > > Cast "const char *" pointers to "char *" to avoid compiler warnings. > > > > 2018-10-01 Christophe Lyon <christophe.lyon@linaro.org> > > > > * newlib/libc/ctype/ctype_.c (__set_ctype): Cast "_ctype_" to "char *". > > * newlib/libc/ctype/jp2uc.c (_jp2uc_l, _uc2jp_l): Cast output of > > "__locale_charset()" and "__current_locale_charset()" to "char *". > > * newlib/libc/locale/locale.c (__loadlocale): Cast "new_locale" to > > "char *". > > > > diff --git a/newlib/libc/ctype/ctype_.c b/newlib/libc/ctype/ctype_.c > > index 28727e8..851fc06 100644 > > --- a/newlib/libc/ctype/ctype_.c > > +++ b/newlib/libc/ctype/ctype_.c > > @@ -176,7 +176,7 @@ __set_ctype (struct __locale_t *loc, const char *charset) > > # if defined(ALLOW_NEGATIVE_CTYPE_INDEX) > > ctype_ptr = _ctype_b; > > # else > > - ctype_ptr = _ctype_; > > + ctype_ptr = (char *) _ctype_; > > # endif > > } > > # if defined(ALLOW_NEGATIVE_CTYPE_INDEX) > > diff --git a/newlib/libc/ctype/jp2uc.c b/newlib/libc/ctype/jp2uc.c > > index b89b5ea..00272eb 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(); > > + char * cs = l ? (char *) __locale_charset(l) : (char *) __current_locale_charset(); > > Why not change cs to const char *? It's only used to call strcmp. > > Probably most other casts should be similarly considered. > > Generally, this patch feels wrong, IMO. > OK, I've split the patch in two parts, then: - one for jp2uc.c following Richard's suggestion - one for the other two files. In locale.c, I add a (char *) cast for new_locale just like what is already done a few lines above for the Cygwin case. In ctype_.c, it's similar in that it adds the cast to effectively behave like in the Cygwin case. I've noticed the comment saying that with Cygwin the values maybe overwritten hence the absence of 'const', but I don't know what happens for non-Cygwin environments: maybe the proper fix would be to remove 'const' when declaring _ctype_ and new_locale ? > R. > > > 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(); > > + char * cs = l ? (char *) __locale_charset(l) : (char *) __current_locale_charset(); > > if (0 == strcmp (cs, "JIS")) > > c = __uc2jp (c, JP_JIS); > > else if (0 == strcmp (cs, "SJIS")) > > diff --git a/newlib/libc/locale/locale.c b/newlib/libc/locale/locale.c > > index 791a775..79da35f 100644 > > --- a/newlib/libc/locale/locale.c > > +++ b/newlib/libc/locale/locale.c > > @@ -515,7 +515,7 @@ restart: > > } > > # define FAIL goto restart > > #else > > - locale = new_locale; > > + locale = (char *) new_locale; > > # define FAIL return NULL > > #endif > > > > > commit 2f4f47d593d6bdbd62510afe4c3c2298615d5762 Author: Christophe Lyon <christophe.lyon@linaro.org> 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 <christophe.lyon@linaro.org> * 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")) commit c6af2b7be0e94935c589307c14eeb9f53901a811 Author: Christophe Lyon <christophe.lyon@linaro.org> Date: Mon Oct 1 18:53:37 2018 +0000 Cast "const char *" pointers to "char *" to avoid compiler warnings. 2018-10-01 Christophe Lyon <christophe.lyon@linaro.org> * newlib/libc/ctype/ctype_.c (__set_ctype): Cast "_ctype_" to "char *". * newlib/libc/locale/locale.c (__loadlocale): Cast "new_locale" to "char *". diff --git a/newlib/libc/ctype/ctype_.c b/newlib/libc/ctype/ctype_.c index 28727e8..851fc06 100644 --- a/newlib/libc/ctype/ctype_.c +++ b/newlib/libc/ctype/ctype_.c @@ -176,7 +176,7 @@ __set_ctype (struct __locale_t *loc, const char *charset) # if defined(ALLOW_NEGATIVE_CTYPE_INDEX) ctype_ptr = _ctype_b; # else - ctype_ptr = _ctype_; + ctype_ptr = (char *) _ctype_; # endif } # if defined(ALLOW_NEGATIVE_CTYPE_INDEX) diff --git a/newlib/libc/locale/locale.c b/newlib/libc/locale/locale.c index 791a775..79da35f 100644 --- a/newlib/libc/locale/locale.c +++ b/newlib/libc/locale/locale.c @@ -515,7 +515,7 @@ restart: } # define FAIL goto restart #else - locale = new_locale; + locale = (char *) new_locale; # define FAIL return NULL #endif
On 05/10/2018 11:32, Christophe Lyon wrote: > commit c6af2b7be0e94935c589307c14eeb9f53901a811 > Author: Christophe Lyon<christophe.lyon@linaro.org> > Date: Mon Oct 1 18:53:37 2018 +0000 > > Cast "const char *" pointers to "char *" to avoid compiler warnings. > > 2018-10-01 Christophe Lyon<christophe.lyon@linaro.org> > > * newlib/libc/ctype/ctype_.c (__set_ctype): Cast "_ctype_" to "char *". > * newlib/libc/locale/locale.c (__loadlocale): Cast "new_locale" to > "char *". There is a __DECONST() macro in <sys/cdefs.h>. I think this better documents the purpose of these casts. It silences also -Wcast-qual warnings. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
On Fri, 5 Oct 2018 at 13:18, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote: > > On 05/10/2018 11:32, Christophe Lyon wrote: > > commit c6af2b7be0e94935c589307c14eeb9f53901a811 > > Author: Christophe Lyon<christophe.lyon@linaro.org> > > Date: Mon Oct 1 18:53:37 2018 +0000 > > > > Cast "const char *" pointers to "char *" to avoid compiler warnings. > > > > 2018-10-01 Christophe Lyon<christophe.lyon@linaro.org> > > > > * newlib/libc/ctype/ctype_.c (__set_ctype): Cast "_ctype_" to "char *". > > * newlib/libc/locale/locale.c (__loadlocale): Cast "new_locale" to > > "char *". > > There is a __DECONST() macro in <sys/cdefs.h>. I think this better > documents the purpose of these casts. It silences also -Wcast-qual warnings. > Hmmm, it seems it isn't used anywhere in the newlib sources, is it really a good idea? I'm not sure it will be clearer... > -- > Sebastian Huber, embedded brains GmbH > > Address : Dornierstr. 4, D-82178 Puchheim, Germany > Phone : +49 89 189 47 41-16 > Fax : +49 89 189 47 41-09 > E-Mail : sebastian.huber@embedded-brains.de > PGP : Public key available on request. > > Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. >
On Oct 2 11:10, Christophe Lyon wrote: > On Tue, 2 Oct 2018 at 00:50, Craig Howland <howland@lgsinnovations.com> 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. Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat
On Wed, 10 Oct 2018 at 11:22, Corinna Vinschen <vinschen@redhat.com> wrote: > > On Oct 2 11:10, Christophe Lyon wrote: > > On Tue, 2 Oct 2018 at 00:50, Craig Howland <howland@lgsinnovations.com> 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 ? Christophe > > Corinna > > -- > Corinna Vinschen > Cygwin Maintainer > Red Hat
On Oct 10 16:37, Christophe Lyon wrote: > On Wed, 10 Oct 2018 at 11:22, Corinna Vinschen <vinschen@redhat.com> wrote: > > > > On Oct 2 11:10, Christophe Lyon wrote: > > > On Tue, 2 Oct 2018 at 00:50, Craig Howland <howland@lgsinnovations.com> 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. Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat
On Wed, 10 Oct 2018 at 17:02, Corinna Vinschen <vinschen@redhat.com> wrote: > > On Oct 10 16:37, Christophe Lyon wrote: > > On Wed, 10 Oct 2018 at 11:22, Corinna Vinschen <vinschen@redhat.com> wrote: > > > > > > On Oct 2 11:10, Christophe Lyon wrote: > > > > On Tue, 2 Oct 2018 at 00:50, Craig Howland <howland@lgsinnovations.com> 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 <christophe.lyon@linaro.org> 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 <christophe.lyon@linaro.org> * 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"))
On Oct 10 22:37, Christophe Lyon wrote: > On Wed, 10 Oct 2018 at 17:02, Corinna Vinschen <vinschen@redhat.com> wrote: > > > > On Oct 10 16:37, Christophe Lyon wrote: > > > 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: Can you please send it in `git format-patch' format? The patch doesn't apply as is. There's also no requirement anymore to add CVS-like commit messages. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat
On Thu, 11 Oct 2018 at 09:11, Corinna Vinschen <vinschen@redhat.com> wrote: > > On Oct 10 22:37, Christophe Lyon wrote: > > On Wed, 10 Oct 2018 at 17:02, Corinna Vinschen <vinschen@redhat.com> wrote: > > > > > > On Oct 10 16:37, Christophe Lyon wrote: > > > > 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: > > Can you please send it in `git format-patch' format? The patch > doesn't apply as is. There's also no requirement anymore to add > CVS-like commit messages. > Is this new version OK? > > Thanks, > Corinna > > -- > Corinna Vinschen > Cygwin Maintainer > Red Hat From dfb509574e1cbbdf4fe9d4d3e8319bd3ffdca98c Mon Sep 17 00:00:00 2001 From: Christophe Lyon <christophe.lyon@linaro.org> Date: Fri, 5 Oct 2018 09:11:05 +0000 Subject: [PATCH] newlib/libc/ctype/jp2uc.c: 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. --- newlib/libc/ctype/jp2uc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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"))
On Oct 11 10:45, Christophe Lyon wrote: > On Thu, 11 Oct 2018 at 09:11, Corinna Vinschen <vinschen@redhat.com> wrote: > > > > On Oct 10 22:37, Christophe Lyon wrote: > > > On Wed, 10 Oct 2018 at 17:02, Corinna Vinschen <vinschen@redhat.com> wrote: > > > > > > > > On Oct 10 16:37, Christophe Lyon wrote: > > > > > 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: > > > > Can you please send it in `git format-patch' format? The patch > > doesn't apply as is. There's also no requirement anymore to add > > CVS-like commit messages. > > > > Is this new version OK? Yep, pushed. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer Red Hat
diff --git a/newlib/libc/ctype/ctype_.c b/newlib/libc/ctype/ctype_.c index 28727e8..851fc06 100644 --- a/newlib/libc/ctype/ctype_.c +++ b/newlib/libc/ctype/ctype_.c @@ -176,7 +176,7 @@ __set_ctype (struct __locale_t *loc, const char *charset) # if defined(ALLOW_NEGATIVE_CTYPE_INDEX) ctype_ptr = _ctype_b; # else - ctype_ptr = _ctype_; + ctype_ptr = (char *) _ctype_; # endif } # if defined(ALLOW_NEGATIVE_CTYPE_INDEX) diff --git a/newlib/libc/ctype/jp2uc.c b/newlib/libc/ctype/jp2uc.c index b89b5ea..00272eb 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(); + char * cs = l ? (char *) __locale_charset(l) : (char *) __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(); + char * cs = l ? (char *) __locale_charset(l) : (char *) __current_locale_charset(); if (0 == strcmp (cs, "JIS")) c = __uc2jp (c, JP_JIS); else if (0 == strcmp (cs, "SJIS")) diff --git a/newlib/libc/locale/locale.c b/newlib/libc/locale/locale.c index 791a775..79da35f 100644 --- a/newlib/libc/locale/locale.c +++ b/newlib/libc/locale/locale.c @@ -515,7 +515,7 @@ restart: } # define FAIL goto restart #else - locale = new_locale; + locale = (char *) new_locale; # define FAIL return NULL #endif
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 commit 349a08c43cd4baf0d93f28ea8ca7351bf9606d50 Author: Christophe Lyon <christophe.lyon@linaro.org> Date: Mon Oct 1 18:53:37 2018 +0000 Cast "const char *" pointers to "char *" to avoid compiler warnings. 2018-10-01 Christophe Lyon <christophe.lyon@linaro.org> * newlib/libc/ctype/ctype_.c (__set_ctype): Cast "_ctype_" to "char *". * newlib/libc/ctype/jp2uc.c (_jp2uc_l, _uc2jp_l): Cast output of "__locale_charset()" and "__current_locale_charset()" to "char *". * newlib/libc/locale/locale.c (__loadlocale): Cast "new_locale" to "char *".