Description: libxls is a C library which can read Excel (xls) files. libxls is also a requirement for building qtiplot 0.9.7.11. SPEC:http://dl.dropbox.com/u/1338197/1/libxls.spec SRPM:http://dl.dropbox.com/u/1338197/1/libxls-0.3.0-1.20100130svn.fc12.src.rpm koji:http://koji.fedoraproject.org/koji/taskinfo?taskID=1953266
* The svn checkout appears to be pre-release snapshot of 0.3.0. Hence the following applies: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages * Can you take a look at the many format string warnings in the build.log? They are reason to be concerned. * The build.log also prints warnings about automake/aclocal, which should be fixed by regenerating the autotools framework prior to packaging the tarball. * I highly recommend to move the %check section _after_ the %install section and to point LD_LIBRARY_PATH at the files in %buildroot. That has helped with discovering packaging mistakes at least a couple of times before. > License: LGPLv2+ * src/getopt.c is not LGPLv2+ but original BSD with advertising clause, which is not GPL compatible according to the licensing matrix. https://fedoraproject.org/wiki/Licensing/BSD#Original_BSD_License_.28BSD_with_advertising.29 http://fedoraproject.org/wiki/Licensing#SoftwareLicenses * All other files in src/ are LGPLv3+.
(In reply to comment #1) > * The svn checkout appears to be pre-release snapshot of 0.3.0. Hence the > following applies: > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages > It seems to be formal release, the upstream released binrary for libxls 0.3.0, but there's no source for libxls 0.3.0. See http://sourceforge.net/projects/libxls/ > * Can you take a look at the many format string warnings in the build.log? They > are reason to be concerned. > > * The build.log also prints warnings about automake/aclocal, which should be > fixed by regenerating the autotools framework prior to packaging the tarball. > > * I highly recommend to move the %check section _after_ the %install section > and to point LD_LIBRARY_PATH at the files in %buildroot. That has helped with > discovering packaging mistakes at least a couple of times before. I'll try to fix those problem. > > > License: LGPLv2+ > > * src/getopt.c is not LGPLv2+ but original BSD with advertising clause, which > is not GPL compatible according to the licensing matrix. > > https://fedoraproject.org/wiki/Licensing/BSD#Original_BSD_License_.28BSD_with_advertising.29 > http://fedoraproject.org/wiki/Licensing#SoftwareLicenses > > * All other files in src/ are LGPLv3+. Should the license for libxls be changed to LGPLv3 and BSD with advertising?
About ormat string warnings: xls.c:123: warning: format '%ld' expects type 'long int', but argument 2 has type 'DWORD' xls.c:151: warning: format '%ld' expects type 'long int', but argument 2 has type 'DWORD' xls.c:176: warning: format '%ld' expects type 'long int', but argument 2 has type 'DWORD' xls.c:224: warning: format '%li' expects type 'long int', but argument 3 has type 'DWORD' xls.c:252: warning: format '% 4ld' expects type 'long int', but argument 2 has I think its we can skip those warnings, it can works fine because "typedef unsigned long DWORD" automake/aclocal are not needed since I didn't modified Makefile.am, acinclude.m4' or configure.in. New SPEC:http://dl.dropbox.com/u/1338197/1/libxls.spec SRPM:http://dl.dropbox.com/u/1338197/1/libxls-0.3.0-2.20100130svn.fc12.src.rpm
> automake/aclocal are not needed since I didn't modified Makefile.am, > acinclude.m4' or configure.in. I refer to regenering the autotools framework _prior to_ creating the source tarball. For example, configure.in is newer than configure. Hence the source tarball is mispackaged and also triggers warnings. > Should the license for libxls be changed to LGPLv3 and BSD with advertising? Yes, "LGPLv3+ and BSD with advertising". Upstream should get rid of src/getopt.c and replace it with something less troublesome. > I think its we can skip those warnings, it can works fine because > "typedef unsigned long DWORD" Where did you find that? Here it uses uint32_t, which is unsigned and 32-bit. So, %u not %ld. There are many more compiler warnings: http://koji.fedoraproject.org/koji/getfile?taskID=1953267&name=build.log A few excerpts with reason to be worried: xls2csv.c:182: warning: comparison with string literal results in unspecified behavior xls.c:390: warning: overflow in implicit constant conversion xlstool.c:447: warning: zero-length gnu_printf format string xlstool.c:456: warning: format '%.*s' expects type 'char *', but argument 4 has type 'WORD *' xlstool.c:489: warning: spurious trailing '%' in format xlstool.c:578: warning: embedded '\0' in format
s/regenering/regenerating/
Perhaps, I should package libxls 0.2.0, version 0.3.0 have some many bugs also there's some API changes between 0.2.0 and 0.3.0. I don't know if version 0.3.0 can work with qtiplot.
Should all warnings in libxls be fixed before making libxls available in fedora? Since upstream are not active now, It will cost a lot of time. I test the libxls in both i686(rhel) and x84_64(ubuntu), it works OK! (In reply to comment #4) > > automake/aclocal are not needed since I didn't modified Makefile.am, > > acinclude.m4' or configure.in. > > I refer to regenering the autotools framework _prior to_ creating the source > tarball. For example, configure.in is newer than configure. Hence the source > tarball is mispackaged and also triggers warnings. > > > > Should the license for libxls be changed to LGPLv3 and BSD with advertising? > > Yes, "LGPLv3+ and BSD with advertising". Upstream should get rid of > src/getopt.c and replace it with something less troublesome. > > > > I think its we can skip those warnings, it can works fine because > > "typedef unsigned long DWORD" > > Where did you find that? > > Here it uses uint32_t, which is unsigned and 32-bit. So, %u not %ld. > > > There are many more compiler warnings: > http://koji.fedoraproject.org/koji/getfile?taskID=1953267&name=build.log > > A few excerpts with reason to be worried: > xls2csv.c:182: warning: comparison with string literal results in unspecified > behavior > xls.c:390: warning: overflow in implicit constant conversion > xlstool.c:447: warning: zero-length gnu_printf format string > xlstool.c:456: warning: format '%.*s' expects type 'char *', but argument 4 has > type 'WORD *' > xlstool.c:489: warning: spurious trailing '%' in format > xlstool.c:578: warning: embedded '\0' in format
By the way, most warnings are useful only in debug mode. The global variable xls_debug is 0 by default. xls.c:123: warning: format '%ld' expects type 'long int', but argument 2 has type 'DWORD' xls.c:151: warning: format '%ld' expects type 'long int', but argument 2 has type 'DWORD' xls.c:176: warning: format '%ld' expects type 'long int', but argument 2 has type 'DWORD' xls.c:224: warning: format '%li' expects type 'long int', but argument 3 has type 'DWORD' xls.c:252: warning: format '% 4ld' expects type 'long int', but argument 2 has 122 if (xls_debug) { 123 printf("xls_appendSST %ld\n", size); 124 } 150 if (xls_debug) { 151 printf("ln=%ld\n", ln); 152 } xlstool.c:447: warning: zero-length gnu_printf format string 446 case 0x201: //BLANK 447 sprintf(ret,""); 448 break; 449 case 0x0BE: //MULBLANK 450 sprintf(ret,""); 451 break; > xlstool.c:456: warning: format '%.*s' expects type 'char *', but argument 4 has > type 'WORD *' 452 case 0x0204: //LABEL (xlslib generates these) 453 lPtr = (WORD *)cell->l; 454 len = *lPtr++; 455 if(pWB->is5ver) { 456 sprintf(ret,"%.*s", len, lPtr); 457 //printf("Found BIFF5 string of len=%d \"%s\"\n", len, ret); 458 } else 459 if((*(char *)lPtr & 0x01) == 0) { 460 sprintf(ret,"%.*s", len, (char *)lPtr + 1); // 1 is the format 461 //printf("Found BIFF8/ASCII string of len=%d \"%s\"\n", len, ret);
> Should all warnings in libxls be fixed before making libxls > available in fedora? Well, you've already shown that you don't like to fix any of these. It's a tiny package with only few compiler warnings. Fixing real bugs reduces the risk that some of these issues create breakage, which leads to bug reports. > 447 sprintf(ret,""); So what? Even that one would be fixed quickly: ret[0] = '\0'; > most warnings are useful only in debug mode. But still you quoted some which have nothing to do with debug output. E.g. the bad sprintfs in API function xls_getfcell. [Btw, they are not protected against buffer overflows eithers. All the developer does is to add limited safety by creating a static 10KiB array to be used by sprintf. That alone is questionable practise.] > I test the libxls in both i686(rhel) and x84_64(ubuntu), it works OK! So, you have a test-case that tests every possible code path of libxls?
(In reply to comment #9) > > Should all warnings in libxls be fixed before making libxls > > available in fedora? > > Well, you've already shown that you don't like to fix any of these. > > It's a tiny package with only few compiler warnings. Fixing real bugs reduces > the risk that some of these issues create breakage, which leads to bug reports. Sorry for my impatience， since libxls are only required by qtiplot, I'll test if libxls 0.3.0 can work with qtiplot. I don't know which version of libxls were used by qtiplot developer, libxls 0.3.0 changes API a lot. If libxls 0.3.0 can work with qtiplot, I'll fix those compile warnings. Otherwise, I'll switch to packaging libxls 0.2.0 which has few compile waring. It doesn't deserve to fix a library that are not used by any program.
Closed now, libxls is buggy, latest qtiplot 0.9.8 don't require it anymore.
*** This bug has been marked as a duplicate of bug 989859 ***