Re: [PATCH v7 28/31] merge-recursive: fix remaining directory rename + dirty overwrite cases

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 5, 2018 at 1:52 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> On Tue, Jan 30, 2018 at 3:25 PM, Elijah Newren <newren@xxxxxxxxx> wrote:

>> +               /*
>> +                * Stupid double negatives in remove_file; it somehow manages
>> +                * to repeatedly mess me up.  So, just for myself:
>> +                *    1) update_wd iff !ren_src_was_dirty.
>> +                *    2) no_wd iff !update_wd
>> +                *    3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
>> +                */
>
> Not sure iff this comment is at the right place and is a good addition to
> the code base.

Fair enough, and I should apologize for letting my frustration come
through there.  However, what if I replaced the first two lines of the
comment with:

"Because the double negatives somehow keep confusing me..."

so that it reads:
        /*
         * Because the double negatives somehow keep confusing me...
         *    1) update_wd iff !ren_src_was_dirty.
         *    2) no_wd iff !update_wd
         *    3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty
         */

Even if my wording was suboptimal, the rest of the comment did seem
pretty important because I messed up the line after the comment
multiple times.  (You'd think that the odds of getting it right should
be 50/50 and that a simple inversion would fix it, so one could only
mess the line up once, but I'm apparently special).  And then I came
back to look at it later and was still confused.  For some reason, I
seem to need the longer explanation.

> However it hints at the underlying issue of a bad API that is provided
> by remove_file ?

I'd definitely agree with that.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux