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

[christos%zoulas.com@localhost (Christos Zoulas)]

On Feb 16,  2:53pm, david%l8s.co.uk@localhost (David Laight) wrote:
-- Subject: Re: CVS commit: src/usr.bin/make

| 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'.

Yes.

| 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.

On the other hand, using size_t for lengths simplifies the code in many
places (no negative checks) and you don't have to cast arguments to calls.

| 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.

This is part of style.

christos