First of all - your mailer is broken, please fix it. It did not put
linux-kernel properly in the To/Cc fields.
Fare> Dear Jes, SUMMARY: The only thing wrong with the byteorder patch
Fare> is long filenames; I chose to make them explicit and long because
Fare> only Linus is entitled to choose definite filename. I suggest
Fare> include/linux/byteorder/little_endian.h and such. *I'm willing to
Fare> make the changes and send them to Linus*...
I personally don't like putting in extra sub-directories under
include/linux, using byteorder_[lb]e.h is perfectly understandable.
However, your summary is not correct. First of all your patch breaks
networking in libcs compiled against your header files - see below.
Fare> LONG ANSWER:
>> I have just been reading the 2.1.71 and I must admit that I am fairly
>> annoyed with the mess of the new byteorder code, to put it in a mild
>> way.
Fare> I'm sorry that you don't like the byteorder code. I've received
Fare> quite positive feedback, actually.
I have only seen one positive reply, and thats the one from Jakub,
however he partially mis-read my first mail. However, everybody else
I've spoken to do not seem to like it and it has already shown up on the
list that the patch breaks userland compiling.
Fare> The byteorder patch doesn't remove *any* single optimization! On
Fare> the contrary, it uniformly brings all the possible generic
Fare> optimizations from all the possible architectures to every other,
Fare> while preserving architecture-dependent optimizations (and they
Fare> only) in architecture-dependent files.
Ok, I read the patch for the _5th_ time now and found the stuff I was
missing. So i was wrong - our stuff is in there, sorry. However it also
shows that its a nightmare to navigate.
Fare> If you see no optimization in an architecture's file, then it
Fare> means there was no arch-dep optimization in 2.1.70 either. I
Fare> invite you to check this with `gcc -S -fverbose-asm -O2 foo.c -o
Fare> foo.s` with foo.c using a few byteorder functions (I can send you
Fare> my own test program, if you want).
Unless you have tested every possible option this proves nothing.
Fare> Plus I've added 64-bit byteswapping *required by UFS* (and future
Fare> ext3?), uniformly for all architectures, as well as (partial)
Fare> vaxendian support (though I haven't received feedback from
Fare> vaxlinux people; they look asleep).
So? 64-bit swapping could have been added very easily to the existing
files. IMHO vax-handling belongs under include/asm-vax.
Fare> Please note that despite new features (64bit, vaxendian, features
Fare> made available in userspace w/o posix namespace pollution), and
Fare> lots of comments, and additional stubs, my patch *reduces* overall
Fare> kernel file size.
Doing it the way I suggested with one `library' header file would have
the same effect.
Fare> Well, I admit *this* is the weak point, but only of superficial
Fare> nature. I hesitated before doing that. The other solution was to
Fare> create a subdirectory for byteorder files. I would have put them
Fare> in asm-generic/ but asm-generic/ isn't generally accessible from
Fare> user programs. I didn't want to make cryptic filenames either
Fare> (like byteorder_le.h, bo_ve.h, bog.h), or lost in the mass
Fare> (little_endian.h, byteorder.h, etc). All in all, it's really up
Fare> to Linus to decide the Right_File_Name(TM) for the common files; I
Fare> made the filename so he could understand what they were, and
Fare> modify the filenames accordingly.
I cannot speak on the behalf of Linus, but every user is allowed to
(IMHO supposed to) select sensible names for his/her files. Just for the
m68k port I know how hard it is to look over all code I receive and I am
fairly certain Linus has a much harder job at this than I have.
>> Second, instead of keeping these gigantic files of swap functions and
>> defines and then let the architetures overwrite these it would be
>> much cleaner to provide a library of generic functions (in fact this
>> would be very small, only 3 in fact, 16, 32 and 64 bit swaps) and
>> then let asm/byteorder use these if there are no optimized ones.
Fare> Problems: should generic files be included before or after
Fare> arch-dep #defines? If the answer is after, they my patch is just
Fare> perfect. If it's before, then arch-dep #defines and optimization
Fare> can't be understood by generic patches, and every architecture
Fare> would have to duplicate huge #define tables as there was earlier,
Fare> which defeats the purpose of code sharing. I admit swab code could
Fare> be split from byteorder generic (vaxendian suggests it, too). Now,
Fare> if you think you can do better, please do!
I already suggested a better way to do this, implement one file with
generic swapping functions using non-conflicting names, say
__gen_swap16() etc. and call it include/linux/byteswap.h or something
like that. People then implement special functions in
include/asm-foo/byteorder.h and/or include byteswap.h and use those
through #defines. This is IMHO much cleaner, much easier to read and
much easier to maintain for the various ports.
Fare> There is not a bit of useless code in the patch (except for the
Fare> #if 1 and #if 0 about whether or not to define htonl and __htonl
Fare> as actual functions, or as macros).
The tons of defines and macros that are not used by the various ports
and they slow down compiling slightly.
The hton[sl]() stuff is horribly broken!
hton[sl]() _must_ be implemented using inline functions, using defines
breaks the networking part of libc. We've already been biten by this
once for the m68k port a long time ago and it struck the Alpha people
not too long ago, though here it was fixed before it became a real
problem afaik.
Jes