Re: [PATCH v2] Caps in not always shift

[Segher Boessenkool <segher@kernel.crashing.org>]

On Thu, Jul 09, 2015 at 08:00:48AM +0200, Thomas Huth wrote:
> >  /**
> > + * Checks if keypos is a latin key
> > + * @param  keypos
> > + * @return -
> > + */
> > +void check_latin(uint8_t keypos)
> > +	if (keypos > KEYP_LATIN_A || keypos < KEYP_LATIN_Z) {
> > +		return true;
> > +	} else {
> > +		return false;
> > +	}
> > +
> 
> That does not look like valid C to me ... "void" return type and
> returning true or false ...

Valid C90, not valid C99 or C11.  Any sane compiler will warn in any
case :-)

> and what about the outermost curly braces?

Yeah, this doesn't compile at all.

> Anyway, you could even write this much more simple without if-statement
> instead:
> 
> bool check_latin(uint8_t keypos)
> {
> 	return keypos > KEYP_LATIN_A || keypos < KEYP_LATIN_Z;
> }

It's a pretty bad name, "check" doesn't give an indication of what the
polarity of the return value is.

I would expect something more like

bool is_latin(uint8_t keypos)
{
	return keypos >= KEYP_LATIN_A && keypos <= KEYP_LATIN_Z;
}

or

bool is_not_latin(uint8_t keypos)
{
	return keypos < KEYP_LATIN_A || keypos > KEYP_LATIN_Z;
}

The proposed code would always return true?


Segher