On Thu, 6 Apr 2006, Dominik Vogt wrote:
On Thu, Apr 06, 2006 at 11:13:03AM +0200, Dominik Vogt wrote:
First, this code from style.c generates a warning:
if (SID_GET_HAS_NAME(s_id) && (fw->style_name == NULL &&
(matchWildcards(SID_GET_NAME(s_id), fw->class.res_class) == TRUE ||
matchWildcards(SID_GET_NAME(s_id), fw->class.res_name) == TRUE ||
matchWildcards(SID_GET_NAME(s_id), fw->name.name) == TRUE)) ||
matchWildcards(SID_GET_NAME(s_id), fw->style_name ) == TRUE)
{
return True;
}
style.c: In function `fw_match_style_id':
style.c:176: warning: suggest parentheses around && within ||
Second, the code may access a NULL pointer style_name if neither
of the first three matchWildcards is true.
I just noticed this warning and logic error myself and were about to
change it.
Also, I think even if style_name is not NULL, the usual style name
matching should be done:
You are probably right. It was just that you said it should use it instead
of the normal matching. However, that probably only creates a lot of more
problems than anything else, so I think that the way you changed it to is
good. I have updated the manpage to reflect the change.
Just some conciderations: The manpage states that styles are search for in
a specific order. I really fail to see how this information is useful
while examining the logic, other than for the face that matchig early will
save some comparitions. Am I missing something?
if (SID_GET_HAS_NAME(s_id))
{
if (matchWildcards(SID_GET_NAME(s_id), fw->class.res_class) ==
TRUE)
{
return True;
}
if (matchWildcards(SID_GET_NAME(s_id), fw->class.res_name) ==
TRUE)
{
return True;
}
if (matchWildcards(SID_GET_NAME(s_id), fw->name.name) == TRUE)
{
return True;
}
if (fw->style_name == NULL &&
matchWildcards(SID_GET_NAME(s_id), fw->class.res_class) ==
TRUE)
{
return True;
}
}
(Note that multi-line conditions with mixed && and || operators
are hard to read. If you happen to edit such a condition, please
split it into separate conditions.)
I see your point and will do that in the future. It should prevent errors
as the one above.
(I have committed this code to CVS, but feel free to change it).
I've committed a copy-paste-error fix (you never compared with
style_name).
/Viktor
