Re: Sparse crash when mixing int and enum in ternary operator

[Christopher Li <sparse@chrisli.org>]

On Wed, Mar 10, 2010 at 8:05 AM, Kamil Dudka <kdudka@redhat.com> wrote:
> On Wed March 10 2010 02:09:22 Pavel Roskin wrote:
>> I wonder is the quirk handling could be skipped for EXPR_SELECT, but I'm
>> fine with it if it's harmless.
>
> That's more likely question for sparse developers.  I have no test-case which
> goes through the EXPR_SELECT path, so that I can't test it actually.

Hi, Sorry for the late reply. I did play with your patch yesterday.
I believe this is the code we are talking about:

+warn_for_enum_conversions(struct expression *expr, struct symbol *type)
+{
+	switch (expr->type) {
+	case EXPR_CONDITIONAL:
+	case EXPR_SELECT:
+		do_warn_for_enum_conversions(expr->cond_true
+					     /* handle the "?:" quirk */
+					     ?: expr->conditional,
+					     type);
+		do_warn_for_enum_conversions(expr->cond_false, type);
+		break;
+
+	default:
+		do_warn_for_enum_conversions(expr, type);
+	}
+}
+

I feel that we shouldn't do special handling for EXPR_CONDITIONAL here.
We should just make the warn_for_enum_conversions more robust.
Special handing EXPR_CONDITIONAL here is just an one off thing.
What if there is a nested EXPR_CONDITIONAL inside the expr->cond_true?
See, your do_warn_for_enum_conversions() still need to handle
EXPR_CONDITIONAL any way. So I argue that we don't need that
special case for EXPR_CONDITIONAL here.

BTW, EXPR_SELECT only appear after expand stage. So you should not
see EXPR_SELECT in the evaluate stage.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html