esp_log_level_set() for a tag is not effective if tag has already logged

slcasner
Posts: 17
Joined: Tue Dec 06, 2016 8:08 pm

esp_log_level_set() for a tag is not effective if tag has already logged

Postby slcasner » Tue Jan 16, 2018 7:00 am

If you try to set a different log level for a tag that has already emitted some log messages, the change won't take effect unless the tag string in the call is exactly the same const char* pointer as is used in the log messages.
The problem is that the code only does a fast search of the cache, meaning that is looks for the tag string address rather than doing a strcmp to accept a match of a different instance of the same string.

So this function would be effective in a very limited case, but if an application implements a command to change log levels for a collection of components within the application it is likely that the tag string will be collected in a buffer of the command parser, it won't be the exact const char* string defined within the component that will be logging with that tag.

For my own fork of v2.1 I modified log.c so that esp_log_level_set() would do the slow search of the cache. I could not merge that code directly to master because of commit cfd95b62c that implemented the fast search of the cache. If I modify the code again on master and issue a PR, might you accept it?

ESP_igrr
Posts: 2072
Joined: Tue Dec 01, 2015 8:37 am

Re: esp_log_level_set() for a tag is not effective if tag has already logged

Postby ESP_igrr » Tue Jan 16, 2018 10:01 am

Looks like a bug, and something we would accept a PR for if you submit it. As far as i understand this means replacing
`if (s_log_cache.tag == tag) {`
with
`if (strcmp(s_log_cache.tag, tag) == 0) {`,
right?

slcasner
Posts: 17
Joined: Tue Dec 06, 2016 8:08 pm

Re: esp_log_level_set() for a tag is not effective if tag has already logged

Postby slcasner » Tue Jan 16, 2018 8:22 pm

Yes, the change to strcmp is the essence of the change. When I implemented this, I followed what was done in get_cached_log_level() to update the item generation as well, but I don't know important that is. I am attaching the diff of my change and will take your advice about whether the generation update should be done. Part of my change was in updating the existing uncached entry; that has already been implemented in cfd95b62c. Also I noticed a bug in a comment in my change: "Set new level from cache" should read "Set new level in cache".
Attachments
log.c.diff.txt
(2.92 KiB) Downloaded 1023 times

slcasner
Posts: 17
Joined: Tue Dec 06, 2016 8:08 pm

Re: esp_log_level_set() for a tag is not effective if tag has already logged

Postby slcasner » Mon Jan 29, 2018 7:59 pm

To close the loop, this problem is being addressed in PR 1557.

Who is online

Users browsing this forum: No registered users and 10 guests