Re: cifs: remove "sockopt" mount option

On Thu, 25 Jun 2009 18:36:16 -0500
Steve French <[email protected]> wrote:

> On Thu, Jun 25, 2009 at 6:28 PM, Jeff Layton<[email protected]> wrote:
> > On Thu, 25 Jun 2009 17:42:43 -0500
> > For the record, I'm also disappointed that the commented out asn1
> > function got merged today too. I'd have preferred to see that go in
> > when it actually becomes useful (and usable).
> 
> I understand your point.  I thought the timing made sense because
> 1) Shirish had just added the missing ASN define for this (so the
> timing of the change to add the code to support parsing a sequence
> made sense)
> 2) I reviewed it and it looked fine
> 3) testing it it worked fine
> 4) I already reviewed it so didn't want to bloat his future patch
> and make it larger with an ASN support function that
> although needed for the kerberos session setup parsing, really
> has nothing to do with kerberos otherwise.
> 

I see no harm in making the second patch that will actually use this
function a little larger. The problem with this logic is of course:
"What if the second patch never materializes?". We're all busy and
things get postponed.

Then code changes happen that make the original function not work
right. Then someone has to go in and fix it (or review the as of yet
unused code). Or worse yet, they end up reimplementing the function and
just ripping out the unused one anyway. Commented out stuff like this
is extra-susceptible to bitrot.

CIFS is rife with examples of this sort of thing. The only sane way to
manage this is not to merge code that isn't actually being used. That's
what the mainline kernel tree usually does. CIFS should follow suit.

-- 
Jeff Layton <[email protected]>