10 more change sets for review

Jeff Darcy jdarcy at redhat.com
Wed Oct 20 15:12:49 UTC 2010


On 10/20/2010 10:40 AM, Jim Meyering wrote:
>> I'm inclined to NAK this one.  In general, code should call
>> malloc/strdup/etc., check the result, and take situationally
>> appropriate actions on failure.  Calling xstrdup etc. was originally a
>> crutch for generated code, and its use in code we can actually fix
>> should be discouraged.  What are the consequences of returning NULL
>> here?  How hard is that to handle in some way better than exit?
> 
> Returning NULL is not an option at present -- it would result in
> a NULL dereference, since the return value can end up being one
> of the inputs to this function.

Could we return a T_INVALID instead?  That might be easier to handle
properly in subsequent parsing or evaluation.

> Handling OOM properly in the parser would be nice
> but avoiding NULL-deref trumped it.  There are a few
> other places where we currently return NULL on failed
> malloc or strdup that need to be examined.

Good idea.

>> I see that you've removed/disabled this, but I don't see anything that
>> replaces its functionality.  What kind of AST do we get when there's a
>> parse error, and how/where  does the evaluator handle it?  I'm not
>> looking for a long tutorial; simple answers will do.
> 
> The only change in this commit is to record parse failure via
> *result == NULL in the policy: rule, rather than using the file-scoped
> global, syntax_error.  No semantic change.

OK, so the AST we get in that case is NULL.  Good to know.  I see that
the replication code that calls parse/eval can handle that.

> However, if your question is really "do we give usefully-precise syntax
> errors", the answer is no.  You get a boolean: parse succeeded.
> That's the yyparse return value.
> 
> It is certainly possible to do much better, but nicely diagnosing invalid
> queries seems like another one of those "nice to have, but it can wait"
> features.

Agreed.


More information about the iwhd-devel mailing list