Обсуждение: New problem with the latest Kubuntu
Hi, Subject is not really true :) With the latest Kubuntu, I got the latest gcc release. Now, when I compile pgAdmin3 from sources, I get a whole bunch of warning messages: ../pgadmin/include/images/splash.xpm:603: warning : deprecated conversion from string constant to "char*" It seems the new (4.2.3) gcc/g++ release throws this kind of warning messages. In fact, a lot of them. Something between 24,000 and 25,000 warnings for pgAdmin3 trunk sources. Compile is finally successful, but I would prefer to be able to get these warnings out. So, I tried to get a look at it. Using "const char*" instead of "char*" seems to fix the issue, but this is getting bigger and bigger. All "char *" variables should be replaced, but also every functions/methods declarations using char* should be replaced with "const char*". And I think wxChar* will also need some tweaking. My C/C++ knowledge is not really helpful here and I will need advice from my fellow hackers :) Is this the right move ? or am I completely wrong ? If I came up with such a big patch, will it be reviewed for inclusion ? Thanks for any tips/comments. Regards. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Guillaume Lelarge a écrit : > [...] > So, I tried to get a look at it. Using "const char*" instead of "char*" > seems to fix the issue, but this is getting bigger and bigger. All "char > *" variables should be replaced, but also every functions/methods > declarations using char* should be replaced with "const char*". And I > think wxChar* will also need some tweaking. > I forgot to say that I found this fix on this page : http://gcc.gnu.org/ml/gcc-help/2006-07/msg00061.html -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
On Thu, May 1, 2008 at 2:10 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Hi, > > Subject is not really true :) With the latest Kubuntu, I got the latest gcc > release. Now, when I compile pgAdmin3 from sources, I get a whole bunch of > warning messages: > > ../pgadmin/include/images/splash.xpm:603: warning : deprecated conversion > from string constant to "char*" > > It seems the new (4.2.3) gcc/g++ release throws this kind of warning > messages. In fact, a lot of them. Something between 24,000 and 25,000 > warnings for pgAdmin3 trunk sources. Compile is finally successful, but I > would prefer to be able to get these warnings out. > > So, I tried to get a look at it. Using "const char*" instead of "char*" > seems to fix the issue, but this is getting bigger and bigger. All "char *" > variables should be replaced, but also every functions/methods declarations > using char* should be replaced with "const char*". And I think wxChar* will > also need some tweaking. > > My C/C++ knowledge is not really helpful here and I will need advice from > my fellow hackers :) Is this the right move ? or am I completely wrong ? If > I came up with such a big patch, will it be reviewed for inclusion ? > > Thanks for any tips/comments. Hmm, well technically that is correct of course - they are const char *'s. My main concern about changing them is that the XPM files are a standard format, so any changes are likely to get overwritten if images are changed, or forgotten in new images. That said, different apps use different var names in them anyway, so we often do have to manually edit them. As I see it we have three choices: - Try using extern c on them (a suggestion from Greg Stark, though we think that only affects linkage). - Silence the warning on the GCC command line. - Fix them. I'm leaning towards fixing them to be honest. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Dave Page a écrit : > [...] > As I see it we have three choices: > > - Try using extern c on them (a suggestion from Greg Stark, though we > think that only affects linkage). > - Silence the warning on the GCC command line. > - Fix them. > > I'm leaning towards fixing them to be honest. > Fixing them seems better. By fixing them, I hope you mean modifying "char*" with "const char*" because that's what I did till now. The patch attached works on trunk. I didn't try it on 1.8 branch but I'll check this ASAP. Thanks. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Вложения
On Thu, May 1, 2008 at 4:03 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Dave Page a écrit : > > > [...] > > > > As I see it we have three choices: > > > > - Try using extern c on them (a suggestion from Greg Stark, though we > > think that only affects linkage). > > - Silence the warning on the GCC command line. > > - Fix them. > > > > I'm leaning towards fixing them to be honest. > > > > > > Fixing them seems better. By fixing them, I hope you mean modifying "char*" > with "const char*" because that's what I did till now. The patch attached > works on trunk. I didn't try it on 1.8 branch but I'll check this ASAP. Yes, that's what I meant. I'm not going to review every last line of this patch - what I suggest we do is apply that to trunk, and simply silence the warning on the 1.8 and edb branches, to lower the risk of breaking them. Sound reasonable? -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Dave Page a écrit : > On Thu, May 1, 2008 at 4:03 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: >> Dave Page a écrit : >> >>> [...] >>> >>> As I see it we have three choices: >>> >>> - Try using extern c on them (a suggestion from Greg Stark, though we >>> think that only affects linkage). >>> - Silence the warning on the GCC command line. >>> - Fix them. >>> >>> I'm leaning towards fixing them to be honest. >>> >>> >> Fixing them seems better. By fixing them, I hope you mean modifying "char*" >> with "const char*" because that's what I did till now. The patch attached >> works on trunk. I didn't try it on 1.8 branch but I'll check this ASAP. > > Yes, that's what I meant. I'm not going to review every last line of > this patch - what I suggest we do is apply that to trunk, and simply > silence the warning on the 1.8 and edb branches, to lower the risk of > breaking them. Sound reasonable? > OK. Should I apply it ? (FYI, it applies great on the 1.8 branch) -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
On Thu, May 1, 2008 at 4:25 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Dave Page a écrit : > > > OK. Should I apply it ? Yes please, to trunk. > (FYI, it applies great on the 1.8 branch) I'm more hesitant to apply it there in case it breaks something. Can we just silence the compiler warning - or are you and others on the list more confident than I? :-) -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Dave Page a écrit : > On Thu, May 1, 2008 at 4:25 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: >> Dave Page a écrit : >> >> >> OK. Should I apply it ? > > Yes please, to trunk. > Done, thanks. >> (FYI, it applies great on the 1.8 branch) > > I'm more hesitant to apply it there in case it breaks something. Can > we just silence the compiler warning - or are you and others on the > list more confident than I? :-) > I'm not that much confident. Tonight, I'll try to look at how I can silence the compiler warning (command line option or something like that). -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Guillaume Lelarge a écrit : > Dave Page a écrit : > [...] >>> (FYI, it applies great on the 1.8 branch) >> >> I'm more hesitant to apply it there in case it breaks something. Can >> we just silence the compiler warning - or are you and others on the >> list more confident than I? :-) >> > > I'm not that much confident. Tonight, I'll try to look at how I can > silence the compiler warning (command line option or something like that). > I don't find any. Perhaps we can wait a bit. If nobody asks for it, there's no urgent need. It'll give us time to test the dev release with my latest commit. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
On Thu, May 1, 2008 at 9:54 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > I don't find any. Perhaps we can wait a bit. If nobody asks for it, there's > no urgent need. It'll give us time to test the dev release with my latest > commit. Does -Wno-write-strings do it? http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Dave Page a écrit : > On Thu, May 1, 2008 at 9:54 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: > >> I don't find any. Perhaps we can wait a bit. If nobody asks for it, there's >> no urgent need. It'll give us time to test the dev release with my latest >> commit. > > Does -Wno-write-strings do it? > > http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html > You're right, it works now. I commited it (1.8 "standard" and EDB branches). Thanks. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
On Fri, May 2, 2008 at 10:19 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > You're right, it works now. I commited it (1.8 "standard" and EDB branches). Cool - thanks. And I should repeat, for the record, feel free to ignore the EDB branch if you like - I don't expect you to patch there as well, though I won't complain if you do :-) -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Dave Page a écrit : > On Fri, May 2, 2008 at 10:19 AM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: >> You're right, it works now. I commited it (1.8 "standard" and EDB branches). > > Cool - thanks. > > And I should repeat, for the record, feel free to ignore the EDB > branch if you like - I don't expect you to patch there as well, though > I won't complain if you do :-) > I have no problem patching the EDB branch (when I'm allowed :) ). -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
On Fri, May 2, 2008 at 10:41 AM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > I have no problem patching the EDB branch (when I'm allowed :) ). > You're always allowed - it's no different from any other stable branch in that sense. It's only really there to make my life easier as I needed to push out some new features for the build used in Postgres Plus earlier than they would have made the next community release. Thanks anyway :-) -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com