About errors in XFS header files and xfsprogs

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

Hello.

The XFS library header files contained in xfsprogs (2.9.3-1) contain
many errors. Run the following command on a Linux system to verify:

gcc -Wall -Wextra -Wfloat-equal -Wbad-function-cast -Wsign-compare
- -Wunreachable-code -Wpointer-arith -Wcast-qual -Wcast-align
- -Wstrict-prototypes -Wformat-security -Wformat-nonliteral
- -Wnested-externs -Wshadow -Wconversion -Wdeclaration-after-statement
- -Wundef -Wpadded -Wredundant-decls -pedantic -c -o xfs_repair.o xfs_repair.c

(you can use any file which #includes the libxfs.h header). The use of 'long long' is less important. The header files should not only be ISO-C-compatible, but even ANSI-C-compatible. This will allow anyone to use them.

Please, make useful public functions available in header files.
Especially platform_check_ismounted() and platform_check_iswritable() and other "platform" functions.

Also, anything that is a part of the public interface, should be in
the "xfs namespace", i.e. start with "xfs_". At least one issue of this type is known: the list_head structure, declared in other files from other libraries. Things like these are very important to fix, because they are making other header files useless.

You shouldn't handle gettext and internationalization in header
files. Let users do it by themselves. Otherwise, a double-defined errors occur for the "_" macro.

Also, a following error occurs in the library:

gcc -g -O2 -DDEBUG -DVERSION=\"2.9.3\"
- -DLOCALEDIR=\"/usr/local/share/locale\" -DPACKAGE=\"xfsprogs\"
- -I./include -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -funsigned-char
- -fno-strict-aliasing -Wall -I. -g -O2 -DNDEBUG -DVERSION=\"2.9.3\"
- -DLOCALEDIR=\"/usr/local/share/locale\" -DPACKAGE=\"xfsprogs\"
- -I../include -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -funsigned-char
- -fno-strict-aliasing -Wall -c xfs_bmap.c -fPIC -DPIC -o .libs/xfs_bmap.o

xfs_bmap.c: In function 'xfs_bmap_add_extent':
xfs_bmap.c:287: warning: implicit declaration of function 'xfs_bmap_check_leaf_extents'

and because of this, we have:

../libxfs/.libs/libxfs.a(xfs_bmap.o)(.text+0x6cf1): In function `xfs_bmap_add_extent':
xfsprogs-2.9.3/libxfs/xfs_bmap.c:287: undefined reference to `xfs_bmap_check_leaf_extents'
collect2: ld returned 1 exit status
gmake[1]: *** [xfs_copy] Error 1

- --
Pozdrawiam/Regards - Bogdan (Linux & FreeDOS)
Kurs asemblera x86 (DOS & Linux): http://rudy.mif.pg.gda.pl/~bogdro
Grupy dyskusyjne o asm: pl.comp.lang.asm alt.pl.asm alt.pl.asm.win32 Rozmawiaj bezpiecznie: www.JabberPL.org Surfuj anonimowo: tor.eff.org -----BEGIN PGP SIGNATURE-----

iD8DBQFG1vBmNTrTaBxW2h4RA49wAKCTVaJ47Fp/hAVGbJd+ewVvbeoxLACeOIrL sslZvDs9bl6QG/W09SwosYw=
=s/eu
-----END PGP SIGNATURE-----

Sais who? The GNU C extensions are very useful for programming, and there is no reason to move to an inferior dialect.

There is no such thing as a public inteface. There's a good reason libxfs is not a shared libary - it's really nothing more but a collection of useful routines for the xfs utilities.

If you think a proper shared library with a defined interface is a good thing you're probably right, and people would welcome you to work on one.

2007/8/30, Christoph Hellwig <[email protected]>:

Do you want to limit xfs to gcc/Linux? Other Unices may use other ccs.

Best
Martin

These headers compile just fine on IRIX, FreeBSD and MacOSX ... the gcc-specific pieces are in platform dependent headers already.

cheers.

--
Nathan

I wouldn't call "padding struct to align..." an error either.

I'm guessing (not 100% sure) all the "warning: ISO C forbids braced-groups within expressions" is from the endian swap calls which eventually gets to __swabXX() #defines with {} in them in swab.h.

Related is the warning "declaration of '__x' shadows a previous local" caused by the beXX_add() functions doing a double __swabXX() call. Can't see an easy solution to these.

"passing argument n of 'func' with different width due to prototype" doesn't seem to be an issue either. Could add lots of casts to hide it.

Redeclaring optind in platform_getoptreset() in linux.h could be fixed.

It is a known issue that libxfs does not build with DEBUG defined. In the libxfs Makefile:

# don't try linking xfs_repair with a debug libxfs. DEBUG = -DNDEBUG

Regards,
Barry.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

NOTE: a copy goes to the mailing list (because of the patch).

Christoph Hellwig pisze:

Sure, I like them too. But I also write programs and like them to be
available to as many, as possible.

If you want xfsprogs compatible only with GCC, you're right. But GCC
is not the only compiler out there and some people using Linux/Unix may use something else.
So there is a reason - portability. Inserting

#ifdef __GNUC__
#endif

around GCC-specific statements isn't really that hard. I'm not asking for compatibility with all architectures (which might require much more work, especially with endianess, if needed). Maybe I'll do these #defines myself in the future.
I myself am using GCC, but others may not. How would you feel if you
had some great piece of software and had to change some 200 lines just to compile it (because it was written, say, for MSVC or Borland with compiler-specific stuff)?

Anything in header files that is usable for the program including
them, one might call a public interface. If libxfs.a had no public interface, xfsprogs would not exist, because the library wouldn't have any functions for them to call.
If two headers define a function with the same name, we have a
conflict. Same goes for preprocessor macros. We have for example:

#define ACTIVE 1

in xfs_copy.c. If any other included header, say <pthread.h> or <signal.h>, #defines the same name, xfs_copy.c will no longer compile. Is this what everybody wants? Of course not. But this is exactly the type of obstacle I've found when trying to use libxfs with libntfs (will patch them too).

Well, I'm writing a XFS utility, but its not only for XFS, hence the
conflicts. Libxfs is becoming a development library, not just a private, internal-use bunch of routines. It is a part of a 'xfsprogs-devel' package, available probably not only for my system. This should make you think that you aren't making the libxfs.a just for yourselves, but also for everyone else. Thus, the library should be designed with careful thinking, which produces less conflicts.
Please, don't think "the user includes our file libxfs.h, so he/she
surely won't include xxx.h, yyy.h and zzz.h". Things just aren't like this.
You can't of course predict all header files the user will include,
and here's where inserting "xfs_" helps. You put everything in your "namespace" using the prefix and assume no other library will use the "xfs_" prefix. Here's where my hopes on libxfs (and partially on libntfs, too) failed, but hopefully we will fix all the problems.

Maybe one day. Now I can help at least a little. Start with applying
the attached patch to the xfsprogs development directory tree (in the top dir, of course). This patch removes the conflicts between libxfs and libntfs (at least those, which I've found). Any further patches will be sent to the mailing list, unless required otherwise.

- --
Pozdrawiam/Regards - Bogdan (Linux & FreeDOS)
Kurs asemblera x86 (DOS & Linux): http://rudy.mif.pg.gda.pl/~bogdro
Grupy dyskusyjne o asm: pl.comp.lang.asm alt.pl.asm alt.pl.asm.win32 Rozmawiaj bezpiecznie: www.JabberPL.org Surfuj anonimowo: tor.eff.org

-----BEGIN PGP SIGNATURE-----

iD8DBQFG2UUANTrTaBxW2h4RA4/YAJ9xehmvenwZhLCUBybV/Qdlxj8faACgkyLG u/VD0b2/xe8+LpopEGo2mgk=
=a7+K
-----END PGP SIGNATURE-----