Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#2495 closed defect (fixed)

Suspicious GCC warnings for AVC driver

Reported by: Mateusz Łoskot Owned by: Daniel Morissette
Priority: normal Milestone: 1.6.0
Component: OGR_SF Version: svn-trunk
Severity: normal Keywords: AVCBin AVCE00
Cc: warmerdam, Mateusz Łoskot, Markus Neteler

Description

Perhaps I'm overreacting, but I'd suggest to consider the following warnings thrown by GCC 4.1.2 as defect and candidates for fix:

  • avc_bin.c
    gcc -fPIC  -g -DDEBUG  -Wall -Wdeclaration-after-statement  -I../shape -I.. -I../.. -I/home/mloskot/dev/gdal/_svn/trunk/gdal/port -I/home/mloskot/dev/gdal/_svn/trunk/gdal/gcore -I/home/mloskot/dev/gdal/_svn/trunk/gdal/alg -I/home/mloskot/dev/gdal/_svn/trunk/gdal/ogr -I/home/mloskot/dev/gdal/_svn/trunk/gdal/ogr/ogrsf_frmts -DOGR_ENABLED -I/home/mloskot/dev/gdal/_svn/trunk/gdal/port  -c -o ../o/avc_bin.o avc_bin.c
    avc_bin.c: In function ‘_AVCBinReadNextTxt’:
    avc_bin.c:1219: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadString’ differ in signedness
    avc_bin.c: In function ‘_AVCBinReadNextPCCoverageTxt’:
    avc_bin.c:1374: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadString’ differ in signedness
    avc_bin.c: In function ‘_AVCBinReadNextArcDir’:
    avc_bin.c:1517: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadString’ differ in signedness
    avc_bin.c:1525: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadString’ differ in signedness
    avc_bin.c:1540: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadBytes’ differ in signedness
    avc_bin.c: In function ‘_AVCBinReadNextArcNit’:
    avc_bin.c:1563: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadString’ differ in signedness
    avc_bin.c:1583: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadString’ differ in signedness
    avc_bin.c: In function ‘_AVCBinReadOpenTable’:
    avc_bin.c:1856: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadBytes’ differ in signedness
    avc_bin.c: In function ‘_AVCBinReadNextTableRec’:
    avc_bin.c:2078: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadString’ differ in signedness
    
  • avc_e00write.c
    gcc -fPIC  -g -DDEBUG  -Wall -Wdeclaration-after-statement  -I../shape -I.. -I../.. -I/home/mloskot/dev/gdal/_svn/trunk/gdal/port -I/home/mloskot/dev/gdal/_svn/trunk/gdal/gcore -I/home/mloskot/dev/gdal/_svn/trunk/gdal/alg -I/home/mloskot/dev/gdal/_svn/trunk/gdal/ogr -I/home/mloskot/dev/gdal/_svn/trunk/gdal/ogr/ogrsf_frmts -DOGR_ENABLED -I/home/mloskot/dev/gdal/_svn/trunk/gdal/port  -c -o ../o/avc_e00write.o avc_e00write.c
    avc_e00write.c: In function ‘AVCE00DeleteCoverage’:
    avc_e00write.c:868: warning: format ‘%s’ expects type ‘char *’, but argument 6 has type ‘char * (*)(int)’
    avc_e00write.c:901: warning: format ‘%s’ expects type ‘char *’, but argument 6 has type ‘char * (*)(int)’
    avc_e00write.c:913: warning: format ‘%s’ expects type ‘char *’, but argument 6 has type ‘char * (*)(int)’
    avc_e00write.c:934: warning: format ‘%s’ expects type ‘char *’, but argument 5 has type ‘char * (*)(int)’
    
  • avc_mbyte.c
    gcc -fPIC  -g -DDEBUG  -Wall -Wdeclaration-after-statement  -I../shape -I.. -I../.. -I/home/mloskot/dev/gdal/_svn/trunk/gdal/port -I/home/mloskot/dev/gdal/_svn/trunk/gdal/gcore -I/home/mloskot/dev/gdal/_svn/trunk/gdal/alg -I/home/mloskot/dev/gdal/_svn/trunk/gdal/ogr -I/home/mloskot/dev/gdal/_svn/trunk/gdal/ogr/ogrsf_frmts -DOGR_ENABLED -I/home/mloskot/dev/gdal/_svn/trunk/gdal/port  -c -o ../o/avc_mbyte.o avc_mbyte.c
    avc_mbyte.c: In function ‘_AVCJapanese2ArcDBCS’:
    avc_mbyte.c:456: warning: pointer targets in return differ in signedness
    avc_mbyte.c: In function ‘_AVCArcDBCS2JapaneseShiftJIS’:
    avc_mbyte.c:520: warning: pointer targets in return differ in signedness
    
  • avc_rawbin.c
    avc_rawbin.c: In function ‘AVCRawBinReadString’:
    avc_rawbin.c:277: warning: pointer targets in passing argument 2 of ‘AVCE00ConvertFromArcDBCS’ differ in signedness
    avc_rawbin.c:277: warning: pointer targets in assignment differ in signedness
    avc_rawbin.c: In function ‘AVCRawBinEOF’:
    avc_rawbin.c:376: warning: pointer targets in passing argument 3 of ‘AVCRawBinReadBytes’ differ in signedness
    

Attachments (2)

avc-ticket-2495.patch (15.9 KB ) - added by Mateusz Łoskot 16 years ago.
Patch for AVCBin and AVCE00 drivers removing GCC warnings. Patch was generated againt SVN r14992. After it s applied, ogr/ogrsf_frmts/avc is supposed to compile without any warnings.
field-prec-fix-ticket-2495.patch (590 bytes ) - added by Mateusz Łoskot 16 years ago.
Patch with tiny fix for warning pointed out by Markus

Download all attachments as: .zip

Change History (17)

comment:1 by warmerdam, 16 years ago

Cc: warmerdam Daniel Morissette added
Component: defaultOGR_SF
Owner: changed from warmerdam to Mateusz Łoskot

Daniel,

Would you like Mateusz to prepare patches resolving these or deal with them yourself or perhaps ignore?

comment:2 by Daniel Morissette, 16 years ago

If Mateusz can provide patches then I'll apply them upstream. If he can't then let me know and I'll add this to my TODO list.

comment:3 by Mateusz Łoskot, 16 years ago

Status: newassigned

Daniel,

I will provide patch and then turn this ticket over to you.

comment:4 by Mateusz Łoskot, 16 years ago

Daniel,

I have additional comment, shouldn't the following two functions return unsigned char instead of signed?

  • _AVCJapanese2ArcDBCS
  • _AVCArcDBCS2JapaneseShiftJIS

comment:5 by Daniel Morissette, 16 years ago

Yes, I think you're right that they should return unsigned char.

comment:6 by Mateusz Łoskot, 16 years ago

Daniel,

OK, working on that.

I have one more issue to clarify. In file avc_e00write.c, I've encountered such construction:

CPLError(CE_Failure, CPLE_FileIO, 
         "Failed deleting %s%s: %s", 
         pszCoverPath, papszFiles[i], strerror);

What is the strerror parameter here? In C library, it's a function but I doubt that strerror pointer is intentionally passed here to CPLerror function, isn't it?

Grepping through GDAL source, I haven't found any global object called strerror.

Could you enlighten me?

by Mateusz Łoskot, 16 years ago

Attachment: avc-ticket-2495.patch added

Patch for AVCBin and AVCE00 drivers removing GCC warnings. Patch was generated againt SVN r14992. After it s applied, ogr/ogrsf_frmts/avc is supposed to compile without any warnings.

comment:7 by Mateusz Łoskot, 16 years ago

Owner: changed from Mateusz Łoskot to Daniel Morissette
Status: assignednew

comment:8 by warmerdam, 16 years ago

Cc: Mateusz Łoskot added; Daniel Morissette removed

comment:9 by Daniel Morissette, 16 years ago

Resolution: fixed
Status: newclosed

Re comment:6, the strerror was a typo, it should have been strerror(errno), but removing the call is also fine to avoid issues in the future.

I have made a few more changes on top of mloskot's patch to change some of the API to use GByte* instead of char* where that made sense, but we still use casts from char to GByte in places where the strings are not expected to be non-ascii.

Committed in AVCE00 CVS upstream and in GDAL SVN r15013 and r15014

comment:10 by Markus Neteler, 16 years ago

Hi,

I got one more (SVN trunk):

gcc -g -O2 -fPIC  -Wall -Wdeclaration-after-statement  -I../shape -I.. -I../.. -I/home/neteler/software/gdal/port -I/home/neteler/software/gdal/gcore -I/home/neteler/software/gdal/alg -I/home/neteler/software/gdal/ogr -I/home/neteler/software/gdal/ogr/ogrsf_frmts -DOGR_ENABLED -I/home/neteler/software/gdal/port  -c -o ../o/avc_e00read.o avc_e00read.c
avc_e00read.c: In function ‘_AVCE00ReadBuildSqueleton’:
avc_e00read.c:975: warning: field precision should have type ‘int’, but argument 3 has type ‘size_t’

Markus

by Mateusz Łoskot, 16 years ago

Patch with tiny fix for warning pointed out by Markus

comment:11 by Markus Neteler, 16 years ago

Cc: Markus Neteler added

comment:12 by Mateusz Łoskot, 16 years ago

Just a tiny fix to frmts/aigrid, required after recent update to AVC drivers (r15018)

comment:13 by Daniel Morissette, 16 years ago

Milestone: 1.6.0

Wow, which version of GCC caught the printf precision warning? It's in a call to *CPLSPrintf()*, how does it know that it takes a printf-like format string? I guess it follows the execution path and sees that the args are passed to vsprintf or something like that... I'm amazed!

Anyway, I've fixed this upstream and in r15019

comment:14 by Mateusz Łoskot, 16 years ago

Daniel,

This feature has been there for long, but it's activated with -Wformat

Try to compile this test.c program:

#include <stdio.h>
int main (void)
{
  printf ("%d", 0.0);
  printf ("%lu", 0u);
  printf ("%lu", sizeof 0);
  return 0;
}

but using these commands:

  1. gcc test.c
  1. gcc -Wformat test.c
  1. gcc -Wall test.c

Yet another argument for building GDAL with -Wall always on :-)

in reply to:  13 comment:15 by Markus Neteler, 16 years ago

Replying to dmorissette:

Wow, which version of GCC caught the printf precision warning? It's in a call to *CPLSPrintf()*, how does it know that it takes a printf-like format string? I guess it follows the execution path and sees that the args are passed to vsprintf or something like that... I'm amazed!

Anyway, I've fixed this upstream and in r15019

It is:

gcc -v
Using built-in specs.
Target: x86_64-mandriva-linux-gnu
Configured with: ../configure --prefix=/usr --libexecdir=/usr/lib --with-slibdir=/lib64 --mandir=/usr/share/man --infodir=/usr/share/info --enable-checking=release --enable-languages=c,c++,ada,fortran,objc,obj-c++,java --host=x86_64-mandriva-linux-gnu --with-cpu=generic --with-system-zlib --enable-threads=posix --enable-shared --enable-long-long --enable-__cxa_atexit --disable-libunwind-exceptions --enable-clocale=gnu --enable-java-awt=gtk --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0/jre --enable-gtk-cairo --disable-libjava-multilib --enable-ssp --disable-libssp
Thread model: posix
gcc version 4.2.2 20071128 (prerelease) (4.2.2-3.1mdv2008.0)
Note: See TracTickets for help on using tickets.