Re: CVS commit: src/usr.bin/make

[David Laight <david%l8s.co.uk@localhost>]

On Fri, Feb 15, 2008 at 09:29:50PM +0000, Christos Zoulas wrote:
> 
> Module Name:  src
> Committed By: christos
> Date:         Fri Feb 15 21:29:50 UTC 2008
> 
> Modified Files:
>       src/usr.bin/make: arch.c buf.c buf.h compat.c cond.c dir.c dir.h for.c
>           job.c job.h main.c make.c make.h nonints.h parse.c str.c suff.c
>           targ.c trace.c util.c var.c
>       src/usr.bin/make/lst.lib: lstForEachFrom.c lstIsAtEnd.c
> 
> Log Message:
> back all changes out until I fix it properly.

I've found out why the change broke things so badly..

JobDoOutput() contains:

       for (i = job->curPos + nr - 1; i >= job->curPos; i--) {
            if (job->outBuf[i] == '\n') {
                gotNL = TRUE;
                break;
            } else if (job->outBuf[i] == '\0') {
                /*
                 * Why?
                 */
               job->outBuf[i] = ' ';
            }
        }

If the buffer being scanned doesn't contain a '\n', and curPos is zero
the we (now) trundle back through memory replacing '\0' with ' '
until we reach a '\n'.

Really this means that a lot of the variables should remain as 'int'.
IMHO just because a variable contains a length doesn't mean it needs
changing to size_t (or ssize_t). If the domain of that specific
variable is small (and most of those changed are), then 'int' ought
to be perfectly acceptable.

I'm also not sure that adding 'integer type' casts on function calls
improves the readability or buglessness of code - indeed since they
can hide bugs (eg actual variable is a pointer) they may hide real bugs.
Adding (void) to functions whose value is ignored also just makes
code harder to read.

        David

-- 
David Laight: david%l8s.co.uk@localhost