xfs
[Top] [All Lists]

Re: [PATCH v3, 14/16] xfsprogs: metadump: fix duplicate handling once an

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH v3, 14/16] xfsprogs: metadump: fix duplicate handling once and for all
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 3 Mar 2011 15:57:14 +1100
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1298657632.1990.6988.camel@doink>
References: <201102182121.p1ILL2Wl029181@xxxxxxxxxxxxxxxxxxxxxx> <20110224083945.GB3166@dastard> <1298657632.1990.6988.camel@doink>
User-agent: Mutt/1.5.20 (2009-06-14)
On Fri, Feb 25, 2011 at 12:13:52PM -0600, Alex Elder wrote:
> On Thu, 2011-02-24 at 19:39 +1100, Dave Chinner wrote:
> > On Fri, Feb 18, 2011 at 03:21:02PM -0600, Alex Elder wrote:
> > > This is a case where I think I've solved a problem to death.
> > 
> > :)
> 
> After getting through the patch, you see what I mean?
> 
> I have some long discussion below.  It is mostly
> explanation for why I ended up with this, so it may
> not convince you it's worth keeping (but I hope so).

It certainly helps understand how you came to this solution, and it
definitely helps explain the _why_ of the code. Hence I think that
if you include the main points from this discussion in in the code,
then it will be OK. Stuff like documenting the change in the number
of alternatives as the length increases, why the bitflip table was
sized and when (if ever) we'd need to consider expanding it, that
handling duplicates of less than 5 characters is not important as we
don't obfuscate names of that length so the low number of alternates
is not an issue, etc.

I know that will make the comments longer than the code, but it
really does need to explain why it has been done this way. That will
save time when someone has to understand it in the future, and I'm
fine with that.....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [PATCH v3, 14/16] xfsprogs: metadump: fix duplicate handling once and for all, Dave Chinner <=