Bug 3589 - openbox/config.c: parse_menu() does unnecessary work
Status: RESOLVED FIXED
Alias: None
Product: Openbox
Classification: Unclassified
Component: general
Version: unspecified
Hardware: All All
: P3 trivial
Assignee: Dana Jansens
QA Contact:
URL:
Depends on:
Blocks:
 
Reported: 2008-04-02 13:37 EDT by KadlSoft
Modified: 2008-04-02 16:28:43 EDT
0 users

See Also:

Attachments
Moves "if"s out of the loop (in parse_menu()) (1.78 KB, patch)
2008-04-02 14:03 EDT, KadlSoft
Move "if"s out of the loop and rewrite the loop to use parse_find_node() to search for "file" (1.96 KB, patch)
2008-04-02 15:25 EDT, KadlSoft
Corrected version of previous patch (1.92 KB, patch)
2008-04-02 16:10 EDT, KadlSoft

Description KadlSoft 2008-04-02 13:37:47 EDT 
Version: 3.4.7-pre3
If you have a look at parse_menu() function (openbox/config.c:787) and parse_find_node() (parse/parse.c:270) you'll realize that both functions loop through list of nodes (xmlNodePtr). That means all "if"s will be evaluated more than once and parse_int() and parse_bool() will be called more times than necessary.
Attached patch moves "if"s out of the loop.
Comment 1 KadlSoft 2008-04-02 14:03:40 EDT 
Created attachment 1718 [details]
Moves "if"s out of the loop (in parse_menu())
Comment 2 KadlSoft 2008-04-02 15:22:08 EDT 
Mikachu suggested to rewrite the loop (that search for "file" node) in parse_menu() to get rid of the ugly xmlChar cast.
Comment 3 KadlSoft 2008-04-02 15:25:04 EDT 
Created attachment 1719 [details]
Move "if"s out of the loop and rewrite the loop to use parse_find_node() to search for "file"
Comment 4 KadlSoft 2008-04-02 16:10:31 EDT 
Created attachment 1720 [details]
Corrected version of previous patch

Previous patch introduced a bug into loop: the loop would find one node more than once.
Comment 5 Mikachu 2008-04-02 16:28:43 EDT 
thanks :)