Re: [hackers] [sbase] Audit expr(1) || FRIGN

From: FRIGN <dev_AT_frign.de>
Date: Mon, 23 Mar 2015 22:43:37 +0100

On Mon, 23 Mar 2015 14:22:19 -0700
Evan Gates <evan.gates_AT_gmail.com> wrote:

Hey Evan,

> If you want to pass structs by pointer instead of value as a style
> change, that's ok. But then you're still using assignment instead of
> memcpy(), well at least in one place, elsewhere you assign each member
> individually. Why? What's wrong with passing/returning/assigning
> structures? It's useful and clear and creates symmetry with builtin
> types, i.e. writing a statement that does the same thing, the same way
> with builtin and user defined types, as opposed to writing it two
> different ways.

Assigning structs is a well-defined feature of the language, I agree.
However, I do not agree making the program more complicated than it
actually is.
Functions like stat() don't return structs but actually take an adress
to a struct. Why? Because the original C didn't support it and because
it's imho unidiomatic.
In the end, structs should not be handled like types. If you have a
problem with that, you can write Java or C++.

> I disagree with getting rid of valstr() and instead duplicating the
> code 4 times and using strings of if else blocks. It makes the code
> longer and more confusing. It also introduces unnecessary allocations
> alongside VLAs e.g. anchreg. And seeing as anchreg is now allocated
> the use of sizeof(anchreg) in estrlcpy and estrlcat is wrong.

Please stop praising VLAs so much. They are an overrated compiler-
feature.
valstr() mixed allocation and static memory-locations and it was not
apparent to the reader what he was dealing with.
In the end, one can always find people for or against a thing like
this. Imho, maybe the val-struct itself should have a field for a
number-string.

> I disagree with adding 0, VAL, (, and ) to the precedence array. They
> are not operators and do not have precedence, and the array is already
> zero filled until the last element which is NE.

This is guaranteed by the standard, yes. However, if you check for
precedence of VAL and the other types, at least explicitly write them in the
prec[]-array. It does no harm and will probably save some readers from being
confused.

> (*valp).str = v.str;
> how about valp->str ... etc.

Yeah, I forgot about that one. Will fix it.

> I disagree with removing all comments in the code, especially the one
> explaining the algorithm used and a link to a better explanation of
> it. The code is fairly simple but the algorithm is not obvious to
> those who have not witnessed it before.

I did not remove all comments in the code, just the kitchen-sinking. One
can expect a programmer to know about Shunting-Yard. This is first semester
knowledge.
We may disagree on this, but I'm a huge proponent of self-documenting code
with exceptions. If there are any troubles in this regard, rewrite the code
instead of writing pages of comments on top of it.

Cheers

FRIGN

-- 
FRIGN <dev_AT_frign.de>
Received on Mon Mar 23 2015 - 22:43:37 CET

This archive was generated by hypermail 2.3.0 : Mon Mar 23 2015 - 22:48:08 CET