Re: rename: move command from util-linux to coreutils

[Jim Meyering]

Sami Kerola wrote:
> On Mon, Jun 11, 2012 at 8:26 AM, Jim Meyering <address@hidden> wrote:
>> Sami Kerola wrote:
>>> On Sun, Jun 10, 2012 at 10:36 PM, Jim Meyering <address@hidden> wrote:
>>>> Here's some quick feedback after a very cursory look:
>>>> I see unchecked syscalls, from write and waitpid to close and dup2.
>>>
>>> I'll correct that to my git tomorrow.
>>>
>>>> Style nits: I saw "TYPE * var_name" in at least one declaration.
>>>> I think it was a parameter list.
>>>> It should be "TYPE *var_name".
>>>
>>> Uh, oh. I cannot trust 'indent -gnu rename.c' fixing everything. I
>>> will fix this at same go with unchecked syscalls.
>>
>> With GNU indent, you have to specify types that are unknown
>> to it via e.g., -Tuintmax_t
>>
>>>> I also saw a TAB or two used in indentation.  Use only spaces.
>>>> If you run "make syntax-check", it will show you where.
>>>
>>> Same for this.
>>
>> Use indent --no-tabs
>>
>>>> Providing more test coverage would be most useful/welcome.
>>>
>>> I will write at least --exec test.
>>
>> Good.  Thanks!
>
> Hi Jim et.al.
>
> My most sincerest apologies, I should not promise do anything during
> weekdays (I said something about delivering fixes at Tuesday). Now at
> the end of weekend there are some fixes in my branch, which are pushed
> with force.
>
>   git://github.com/kerolasa/coreutils.git rename
>
> * Attempt to find, and do something, for all system calls which return
> value was not checked.
> * Style fixes, e.g. pointer types, tabs etc.
> * All issues 'make syntax-check' reported are fixed.
> * There is a very basic --exec option test.
>
> Hopefully the change is now closer to ready.

Hi Sami,

Thanks for the fixes.
There is too much new and untested code to include this new program
in the upcoming release.

A few things should be changed regardless.
For consistency with other GNU tools:

  s/--test/--dry-run/

Also, please ensure that your new code compiles
when configure with --enable-gcc-warnings.
Currently I see several warnings:

  ...
  rename.c: In function 'construct_dest_name':
  rename.c:122:34: error: declaration of 'replacement' shadows a global 
declaration [-Werror=shadow]
                        const char *replacement)
                                    ^
  rename.c:71:14: error: shadowed declaration is here [-Werror=shadow]
   static char *replacement;
                ^
  rename.c: In function 'main':
  rename.c:446:24: error: assignment discards 'const' qualifier from pointer 
target type [-Werror]
               files_from = "-";
                          ^
  rename.c:458:24: error: assignment discards 'const' qualifier from pointer 
target type [-Werror]
               files_from = "-";
                          ^
  rename.c:527:18: error: assignment discards 'const' qualifier from pointer 
target type [-Werror]
         files_from = "-";
                    ^
  rename.c: In function 'request_child':
  rename.c:234:18: error: ignoring return value of 'fgets_unlocked', declared 
with attribute warn_unused_result [-Werror=unused-result]
     fgets (ret + skip, PATH_MAX - skip, mother_file);
                    ^
  cc1: all warnings being treated as errors
  make[3]: *** [rename.o] Error 1
  make[3]: *** Waiting for unfinished jobs....
  make[3]: Leaving directory `/h/j/w/co/cu-sami-kerola-rename/src'
  make[2]: *** [all] Error 2
  make[2]: Leaving directory `/h/j/w/co/cu-sami-kerola-rename/src'
  make[1]: *** [all-recursive] Error 1
  make[1]: Leaving directory `/h/j/w/co/cu-sami-kerola-rename'
  make: *** [all] Error 2