Re: [Sc2-devel] Look Ma, no compiler warnings!

[Serge v. d. B. <sv...@st...>]

On Sun, 29 Dec 2002, Steven Barker wrote:
> > > > Putting an external function declaration inside a function is not good
> > > > practice. The right solution would be to include the correct .h file.
> > > I think the function the declaration is inside of is unused (the comments
> > > claim it's "unimplemented").  The warning was for a redeclaration of the
> > > function, as GetUNICODEKey is defined elsewhere in the file.  This is
> > > definately the right fix for it, unless we want to remove the
> > > "unimplemented" functions entirely.
> > 'unimplemented' meant what it says. I don't know why that remark is still
> > there, maybe it's not completely implemented yet. Anyhow, the input code is
> > being revised, this should be taken care of in time.
> > For now, I'll leave it as it is.
> My patch is exceedingly minimal, as all it does is remove the extra
> function declaration (which never should have been put inside another
> function to begin with).  Even if it's totally unused code, please consider
> applying the patch just to get rid of the silly compiler warning (which
> might distract other people from real bugs).
My mistake. I thought you were _adding_ a prototype within a function,
instead of removing it. Patch appleid.

> > > > Your WRAP_VAL patch is downright wrong. (take a look at the actual
> > > > definition of the macro).
> > > I did.  Its basically a mod operator?  Its common cases are equivelant to
> > > (((COUNT)(v)) % (d)), right?
> > Actually, % returns a negative value when the first argument is negative.
> > It's something every programming language does differently. But I understand
> > what you mean.
> Yes, I know, C is stupid like that.  My code above actually does it right,
> since it casts to an unsigned type (COUNT) before doing the % operator.
I wouldn't necessarilly call it stupid. It's just a design decision.

> > > (I assume it's not ever used where v < -d or v >= 2*d.)
> > Certainly that should be checked instead of assumed.
> Well, the current WRAP_VAL function doesn't do any checks, and will have
> unexpected results for values outside the normal range.  My equivilant code
> above is more consistant (it will always return a value between 0 and w-1).
By 'it should be checked', I didn't mean in the code, but by the person
creating the code.

> [ long story about WRAP_VAL ]
Ah. I didn't get your point that WRAP_VAL itself was used incorrectly in the
original code. Patch applied.

Serge