Hi,
I found the comment on query_id_enabled looks inaccurate because this is
never set to true when compute_query_id is ON.
/* True when compute_query_id is ON, or AUTO and a module requests them */
bool query_id_enabled = false;
Should we fix this as following (just fixing the place of a comma) ?
/* True when compute_query_id is ON or AUTO, and a module requests them */
Also, I think the name is a bit confusing for the same reason, that is,
query_id_enabled may be false even when query id is computed in fact.
Actually, this does not matter because we use IsQueryIdEnabled to check
if query id is enabled, instead of referring to a global variable
(query_id_enabled or compute_query_id). But, just for making a code a bit
more readable, how about renaming this to query_id_required which seems to
stand for the meaning more correctly?
I attached a patch for above fixes.
Although renaming might not be acceptable since changing global variables
may affect third party tools, I think the comment should be fixed at least.
IMHO, it seems better to make this variable static not to be accessed directly.
However, I left it as is because this is used in a static inline function.
Regards,
Yugo Nagata
--
Yugo NAGATA <nagata@sraoss.co.jp>
