Hi all!
The following files emits warnings when compiled with gcc 4.0:
al175.c
bcmxcp_ser.c
belkinunv.c
cyberpower.c
everups.c
powercom.c
solis.c
All warnings seem to be of this variety:
everups.c:38: warning: pointer targets in passing argument 2 of
'ser_get_char' differ in signedness
I suggest that those who fiddles with those drivers fixes the warnings
and verifies that it works afterwards. I can provide the complete
buildlog if there's anyone that doesn't have access to a gcc4-equipped
machine.
/Nikke - prefers a noise free build output :)
--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se | nikke@acc.umu.se
---------------------------------------------------------------------------
"42? 7 and a half million years and all you can come up with is
42?!"
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Done, for belkinunv.c. -- Peter Niklas Edmundsson wrote:> > Hi all! > > The following files emits warnings when compiled with gcc 4.0: > al175.c > bcmxcp_ser.c > belkinunv.c > cyberpower.c > everups.c > powercom.c > solis.c > > All warnings seem to be of this variety: > everups.c:38: warning: pointer targets in passing argument 2 of 'ser_get_char' differ in signedness > > I suggest that those who fiddles with those drivers fixes the warnings > and verifies that it works afterwards. I can provide the complete > buildlog if there's anyone that doesn't have access to a gcc4-equipped > machine. > > /Nikke - prefers a noise free build output :) > -- > -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- > Niklas Edmundsson, Admin @ {acc,hpc2n}.umu.se | nikke@acc.umu.se > --------------------------------------------------------------------------- > "42? 7 and a half million years and all you can come up with is 42?!" > =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-> > _______________________________________________ > Nut-upsdev mailing list > Nut-upsdev@lists.alioth.debian.org > http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev >
> The following files emits warnings when compiled with gcc 4.0: > al175.c > bcmxcp_ser.c > belkinunv.c > cyberpower.c > everups.c > powercom.c > solis.c > > All warnings seem to be of this variety: > everups.c:38: warning: pointer targets in passing argument 2 of > 'ser_get_char' differ in signednessThis is due to the fact that some drivers need to pass character values between 0x80 and 0xFF to or from the drivers, but the serial functions use signed chars. Unless some drivers use negative values, my preference would be to fix/create serial functions (to use unsigned chars), rather than to convert the characters between 0x80 and 0xFF to negative numbers. Especially where status bits are used, listing a hex value is more readable then.> I suggest that those who fiddles with those drivers fixes the warningsand verifies that it works afterwards. I can provide the complete buildlog if there's anyone that doesn't have access to a gcc4-equipped machine. Not needed, gcc will also complain about these problems when you pass it the '-pedantic' option (which is highly recommended when writing portable code). This will also spot the places where C++ style comments are used, which violates the NUT coding standard. Regards, Arjen -- Eindhoven - The Netherlands Key fingerprint - 66 4E 03 2C 9D B5 CB 9B 7A FE 7E C1 EE 88 BC 57
Done for bcmxcp_ser.c. /Kjell tor 2006-02-09 klockan 16:36 +0100 skrev Niklas Edmundsson:> Hi all! > > The following files emits warnings when compiled with gcc 4.0: > al175.c > bcmxcp_ser.c > belkinunv.c > cyberpower.c > everups.c > powercom.c > solis.c > > All warnings seem to be of this variety: > everups.c:38: warning: pointer targets in passing argument 2 of 'ser_get_char' differ in signedness > > I suggest that those who fiddles with those drivers fixes the warnings > and verifies that it works afterwards. I can provide the complete > buildlog if there's anyone that doesn't have access to a gcc4-equipped > machine. > > /Nikke - prefers a noise free build output :)
On 2/9/06, Arjen de Korte <nut+devel@de-korte.org> wrote:> This is due to the fact that some drivers need to pass character values > between 0x80 and 0xFF to or from the drivers, but the serial functions use > signed chars. Unless some drivers use negative values, my preference would > be to fix/create serial functions (to use unsigned chars), rather than to > convert the characters between 0x80 and 0xFF to negative numbers. > Especially where status bits are used, listing a hex value is more > readable then.I tend to agree with Arjen on this one - most drivers will be treating characters as unsigned, since the majority of the values being returned are unsigned. The only exceptions I know about are the ones that Peter mentioned in libHID, and that code is a mess anyway. Is there a reason why the common serial functions should return signed characters? -- - Charles Lepple
On Thu, Feb 09, 2006 at 04:36:00PM +0100, Niklas Edmundsson wrote:> The following files emits warnings when compiled with gcc 4.0: > al175.c > bcmxcp_ser.c > belkinunv.c > cyberpower.c > everups.c > powercom.c > solis.cNow we are at it, could someone re-import the nitram driver with proper line endings? My trusty gcc 2.95 gets hiccups from the continuationlines with the added \r Also, the al175 has some c++ style declarations in the middle of the code blocks and some non-conforming %lf format specifiers, and gendb has a csh style redirection that a true sh does not like. I have attached patches for those. /Niels -- Niels Baggesen - @home - ?rhus - Denmark - nba@users.sourceforge.net The purpose of computing is insight, not numbers --- R W Hamming -------------- next part -------------- Index: drivers/al175.c ==================================================================RCS file: /cvsroot/nut/nut/drivers/Attic/al175.c,v retrieving revision 1.1.2.2 diff -u -r1.1.2.2 al175.c --- drivers/al175.c 10 Oct 2005 19:43:54 -0000 1.1.2.2 +++ drivers/al175.c 8 Feb 2006 14:55:40 -0000 @@ -476,11 +476,12 @@ static int comli_check_frame(const_raw_data_t f) { int bcc; + byte_t *tail; if (*f.begin!=STX) return -1; - byte_t *tail = f.end - 2; + tail = f.end - 2; if (tail <= f.begin) return -1; @@ -811,6 +812,7 @@ static int recv_command_ack() { int err; + raw_data_t ack; /* 1: STX */ err = scan_for(STX); @@ -818,7 +820,7 @@ return -1; - raw_data_t ack = raw_alloca(8); + ack = raw_alloca(8); *(ack.end++) = STX; @@ -857,14 +859,16 @@ static int recv_register_data(io_head_t *io, raw_data_t *io_buf) { int err; + raw_data_t reply_head; + raw_data_t reply; /* 1: STX */ err = scan_for(STX); if (err==-1) return -1; - raw_data_t reply_head = raw_alloca(11); - *(reply_head.end++) = STX; + reply_head = raw_alloca(11); + *(reply_head.end++) = STX; /* 2: ID1 ID2 STAMP MSG_TYPE ADDR1 ADDR2 ADDR3 ADDR4 LEN1 LEN2 */ @@ -894,7 +898,7 @@ #endif /* 4: allocate space for full reply and copy header there */ - raw_data_t reply = raw_alloca(11/*head*/ + io->len/*data*/ + 2/*ETX BCC*/); + reply = raw_alloca(11/*head*/ + io->len/*data*/ + 2/*ETX BCC*/); memcpy(reply.end, reply_head.begin, (reply_head.end - reply_head.begin)); reply.end += (reply_head.end - reply_head.begin); @@ -969,6 +973,8 @@ { int err; raw_data_t REQ_frame = raw_alloca(512); + raw_data_t rx_data; + io_head_t io; al_prep_read_req(&REQ_frame, addr, count); @@ -979,15 +985,10 @@ return -1; - raw_data_t rx_data = { - .buf = dst, - .buf_size = count, - - .begin = dst, - .end = dst - }; - - io_head_t io; + rx_data.buf = dst; + rx_data.buf_size = count; + rx_data.begin = dst; + rx_data.end = dst; err = recv_register_data(&io, &rx_data); if (err==-1) @@ -1195,10 +1196,10 @@ // 0x4100 BATTERY VOLTAGE REF - dstate_setinfo("battery.voltage.nominal", "%.2lf", d16(x4100+0)); + dstate_setinfo("battery.voltage.nominal", "%.2f", d16(x4100+0)); // 0x4110 BOOST VOLTAGE REF - dstate_setinfo("input.transfer.boost.low", "%.2lf", d16(x4100+2)); // XXX: boost.high ? + dstate_setinfo("input.transfer.boost.low", "%.2f", d16(x4100+2)); // XXX: boost.high ? // 0x4120 HIGH BATT VOLT REF XXX // 0x4130 LOW BATT VOLT REF XXX @@ -1206,13 +1207,13 @@ // 0x4180 FLOAT VOLTAGE XXX // 0x4190 BATT CURRENT batt_current *= d16(x4180+2); - dstate_setinfo("battery.current", "%.2lf", batt_current); + dstate_setinfo("battery.current", "%.2f", batt_current); // 0x41b0 LOAD CURRENT (output.current in NUT) - dstate_setinfo("output.current", "%.2lf", d16(x4180+6)); + dstate_setinfo("output.current", "%.2f", d16(x4180+6)); // 0x4300 BATTERY TEMPERATURE - dstate_setinfo("battery.temperature", "%.2lf", d16(x4300+0)); + dstate_setinfo("battery.temperature", "%.2f", d16(x4300+0)); status_commit(); Index: drivers/gendb ==================================================================RCS file: /cvsroot/nut/nut/drivers/gendb,v retrieving revision 1.1.1.1.8.1.2.6 diff -u -r1.1.1.1.8.1.2.6 gendb --- drivers/gendb 19 Sep 2005 16:12:44 -0000 1.1.1.1.8.1.2.6 +++ drivers/gendb 8 Feb 2006 14:55:40 -0000 @@ -10,7 +10,7 @@ MFDB=Makefile.drvbuild -which gcc >& /dev/null +which gcc >/dev/null 2>&1 if [ $? != 0 ]; then echo "$0: gcc not found. Bailing out (and keeping $MFDB intact)." exit 1
Bump this to the list: fre 2006-02-10 klockan 18:28 -0400 skrev Peter Selinger:> OK, that "unsigned char" patch is now committed. As a result, the > following drivers are in need of attention to reduce the gcc4 warning > clutter: > > belkin.c > cyberpower.c > tripplite.c > etapro.c > tripplitesu.c > metasys.c > bestfcom.c > solis.c > al175.c > > I suggest that the respective maintainers have a look and adjust the > types as necessary, making sure the drivers still work. In each case, > an easy solution is to put typecasts on certain function calls; but in > many cases, it makes sense to simply declare variables as unsigned > char. > > -- Peter > > > > fre 2006-02-10 klockan 11:29 -0400 skrev Peter Selinger: > > > Gentlemen, > > > > > > attached my proposed changes. I figured, since we discussed them > for > > > so long, it might be a good idea to let you see them before I > commit. > > > [...] >