Re: coreutils 6.6 c89 build failures

[Jim Meyering]

Paul Eggert <address@hidden> wrote:
> Matthew Woehlke <address@hidden> writes:
>> Is it really so painful to support c89

It is counterproductive.

>> when the needed changes are so trivial
>
> But they make the code uglier and harder to maintain.
>
> However, I think most of those problems can be solved by rewriting the
> code in a not-so-trivial way.  Here's my attempt to do it.  It does
> shrink the overall source code length by 24 lines (including the
> change to Makefile.maint and README, but not counting the removal of
> src/c99-to-c89.diff), so one could argue that this patch maintains
> readability.  (Also, it shrinks the executable text size of 'rm' by a
> whopping 96 bytes (0.25%) on my host (x86, Debian stable, GCC 4.1.1,
> compiled with -O).  :-)
>
> It's really up to Jim, though.  He might not want to accept a patch
> that merely shuffles code around without fixing any real bugs, as the
> patch itself may introduce a bug.

Thanks for taking the time to do all of that.
I do like most of your changes to remove.c, but not those that add
the else clause increase the indentation level, in rm_1.
On the other hand, I prefer the current rm.c to the proposed version.
Similarly, with shred.c, I prefer the current version, because I think
the precondition-style "verify" assertion belongs before the code
that it can be seen as guarding.  Your patch makes a similar
compromise in moving the assertion in remove.c's AD_stack_pop.

This comes down to my opinion that we should all be moving to a language
(C99) that is more suitable to making the code readable and maintainable.
We shouldn't have to jump through hoops to avoid declarations after
statements.  To that end, I want to keep the framework to support
that: the file, c99-to-c89.diff, the patch-check rule, etc.  Then, we
will feel freer (at least I will) to introduce new C99'isms, as these
programs evolve.

Besides, rm is mature enough (and complicated enough) that we should
divide this patch up into smaller, trivially-verifiable-correct, pieces, too.
For example, removing the "int fd_cwd = AT_FDCWD;" statement
and replacing each use of fd_cwd with "AT_FDCWD" would be a single delta.
Similarly, the change that gives cache_stat_init a return type belongs
in a delta by itself.  Likewise for the change that makes AD_pop_and_chdir
return a name.

You can do that easily with git.
First, create a temporary branch:

  git-checkout -b rm-c89

Then make and commit each change individually.
Then, when you've done them all (assuming there were 7), run this
to produce patches you can mail -- and that I can easily import
into my git tree:

  git-format-patch -k --stdout HEAD~7 > log

Or better still, point me to your git repository (assuming I can
get to it), and I'll just "pull" those changes from it.

FYI, I've gone ahead and done the above for you, as an exercise.
In the process, I changed a function name: s/insure/ensure/.
For each delta, I'd make the change, update ChangeLog, then type
"vc-dwim"[*] to inspect/verify the diffs, and if ok, would then
type "vc-dwim --c" to check them in to the private branch.

Jim

[*] For those who don't know, vc-dwim is the tool I use to
perform most of my git, cvs, hg, svn commit and diff commands.
Here is some description along with the latest announcement:
    http://article.gmane.org/gmane.comp.lib.gnulib.bugs/7797