[PATCH 2/2] vi: don't touch file with :x when modified_count == 0

[Kang-Che Sung]

(Mail resent again to CC to busybox mailing list)

On Wed, Feb 22, 2017 at 10:03 PM, walter harms <wharms at bfs.de> wrote:
>
>
> Am 22.02.2017 14:11, schrieb Yousong Zhou:
>> @@ -1034,7 +1034,9 @@ static void colon(char *buf)
>>        || strncmp(p, "wn", cnt) == 0
>>        || (p[0] == 'x' && !p[1])
>>       ) {
>> -             cnt = file_write(current_filename, text, end - 1);
>> +             if (modified_count != 0 || p[0] != 'x') {
>> +                     cnt = file_write(current_filename, text, end - 1);
>> +             }
>>               if (cnt < 0) {
>>                       if (cnt == -1)
>>                               status_line_bold("Write error: %s", strerror(errno));
>> @@ -1045,8 +1047,9 @@ static void colon(char *buf)
>>                               current_filename,
>>                               count_lines(text, end - 1), cnt
>>                       );
>> -                     if (p[0] == 'x' || p[1] == 'q' || p[1] == 'n'
>> -                      || p[0] == 'X' || p[1] == 'Q' || p[1] == 'N'
>> +                     if (p[0] == 'x'
>> +                      || p[1] == 'q' || p[1] == 'n'
>> +                      || p[1] == 'Q' || p[1] == 'N'
>>                       ) {
>>                               editing = 0;
>>                       }
>
>         maybe you can save a few bit by using:
>         if ( strchr("xqnQN",p[0]) )
>                 editing = 0;
>

I think that optimization should be addressed in the separate patch
because for me it isn't easy to see if the behavior matches.

And note there are p[0] and p[1] in the original conditional, but your
strchr() approach uses only p[0]. Is that intentional?


>> @@ -1476,16 +1479,19 @@ static void colon(char *buf)
>>                       goto ret;
>>               }
>>  #endif
>> -             // how many lines in text[]?
>> -             li = count_lines(q, r);
>> -             size = r - q + 1;
>>               //if (useforce) {
>>                       // if "fn" is not write-able, chmod u+w
>>                       // sprintf(syscmd, "chmod u+w %s", fn);
>>                       // system(syscmd);
>>                       // forced = TRUE;
>>               //}
>> -             l = file_write(fn, q, r);
>> +             if (modified_count != 0 || cmd[0] != 'x') {
>> +                     size = r - q + 1;
>> +                     l = file_write(fn, q, r);
>> +             } else {
>> +                     size = 0;
>> +                     l = 0;
>> +             }
>
> you can turn this an its head an simplify you life:
>         size = 0;
>         l = 0;
>         if (modified_count != 0 || cmd[0] != 'x') {
>                 size = r - q + 1;
>                 l = file_write(fn, q, r);
>         }

Do note that the compiler can arrange the blocks so this suggestion
won't save bytes in compiled code.

>> +                             if (q == text && q + l == end) {
>> +                                     modified_count = 0;
>> +                                     last_modified_count = -1;
>> +                             }
>> +                             if (cmd[0] == 'x'
>> +                              || cmd[1] == 'q' || cmd[1] == 'n'
>> +                              || cmd[1] == 'Q' || cmd[1] == 'N'
>> +                             ) {
>> +                                     editing = 0;
>> +                             }
>                 strchr("xqnQN",cmd[0])

Likewise. I can't see the behavior of strchr() approach is same as original.