Christoph Hellwig
2007-Dec-31 09:17 UTC
[Ocfs2-devel] [RFC][PATCH] byteorder: introduce le32_add_cpu & friends to core
On Sun, Dec 30, 2007 at 10:49:34PM +0100, Marcin Slusarz wrote:> I found it in XFS only. Did I miss something?These helpers come from ocfs2 which has both be and le variants.> be32_add is shorter than be32_add_cpu but I think it's not clear > whether second parameter is in native byte order or not.Then again adding foreing endian values doesn't make much sense. If you insist on your naming please make sure to convert ocfs and xfs to your naming.
Mark Fasheh
2007-Dec-31 11:39 UTC
[Ocfs2-devel] Re: [RFC][PATCH] byteorder: introduce le32_add_cpu & friends to core
On Sun, Dec 30, 2007 at 08:06:34PM +0100, Marcin Slusarz wrote:> There are many places where these functions would be useful. > (just look at: grep -r 'cpu_to_[ble12346]*([ble12346]*_to_cpu.*[-+]' linux-src/) > What do you think? > > ps: this patch depends on http://lkml.org/lkml/2007/12/25/35 > -- > > add inline functions which add native byte order variable to > little/big endian variable to core header and as an example > convert ext3 to use themYou might want to note where you got these functions from in your message. Also, it would be easier to follow these changes if you had broken things up into two patches - one which moved stuff from ocfs2 into generic helpers and a 2nd one to convert ext3. Looking into my crystal ball, I see a bunch of "convert to using byteorder math macros" patches coming down the pipe. Since we're talking about disk fields where a mistake could be costly, I suggest that any patch more than a couple of lines should be tested by the submitter with sparse. A statement that sparse didn't produce any new warnings (with the patch applied) could be included in the description. --Mark -- Mark Fasheh Principal Software Developer, Oracle mark.fasheh@oracle.com
Mark Fasheh
2007-Dec-31 11:43 UTC
[Ocfs2-devel] Re: [RFC][PATCH] byteorder: introduce le32_add_cpu & friends to core
On Mon, Dec 31, 2007 at 09:17:32AM +0000, Christoph Hellwig wrote:> > be32_add is shorter than be32_add_cpu but I think it's not clear > > whether second parameter is in native byte order or not. > > Then again adding foreing endian values doesn't make much sense. > > If you insist on your naming please make sure to convert ocfs and xfs > to your naming.Personally, I'd rather stick with the (IMHO) more descriptive name - btw, the ones he picked don't require any extra conversion in Ocfs2. --Mark -- Mark Fasheh Principal Software Developer, Oracle mark.fasheh@oracle.com