On 07/02/14 00:03, Andrew Gregory wrote:
On 07/01/14 at 08:27pm, Olivier Brunel wrote:
On 07/01/14 06:56, Allan McRae wrote:
On 29/06/14 21:35, Olivier Brunel wrote:
On 06/29/14 03:50, Allan McRae wrote:
On 24/06/14 20:18, Olivier Brunel wrote:
On 06/24/14 08:14, Allan McRae wrote: > On 26/05/14 01:30, Andrew Gregory wrote: >> The earlier changes to the event callback have the same issue and also >> give errors about using non-literal strings with vprintf: >> >> testdb.c:53:11: error: format string is not a string literal >> [-Werror,-Wformat-nonliteral] >> vprintf(event->fmt, event->args); >> ^~~~~~~~~~ > > BAH! Why did gcc no see this? > > Unless someone comes up with something else, I'll do the revert soon.
I can't think of another way to silence the warning, but there's a way to "work around" it: instead of putting fmt & args inside alpm_event_log_t, we put the resulting string (i.e. call pm_asprintf() in alpm and send the resulting string in the event).
No more false warnings then (but a malloc/free for each log event). I can send a patch if you'd like.
Can not do that. Translations of messages happen in the front-end.
um, no? Translations obviously happen in ALPM, the call to gettext happens before calling _alpm_log(), so by then whether we send the message to the front-end as a pointer to the full string, or a pointer to a format string and a va_list, doesn't really matter.
All the front-end does is decide whether or not (e.g. based on level), and how, to show the message to the user.
Or did I not understand what you meant at all?
Some parts are, some are not (the prefixes). Also, pm_asprintf is not available in the backend.
I assume by prefixes you mean the front-end could add e.g. "warning:" before the message itself? Then sure, that part would be translated in the front-end, but I fail to see how getting the message as a string and not a format string & va_list would change/break anything?
Either way, the front-end has no idea what the message is, nor does it care/do anything other than showing it to the user.
(And pm_asprintf could simply be moved to util-common.c no?)
I am in favour of reverting the log callback as the solution here.
I feel it's a little sad there isn't a way to silence this false-positive warning, since this is really all it's about.
I guess the log already had its own callback separated from events, although looking at the history it seems more to be because the log_cb was created first than for any actual reason, and "merging it" into events was a good idea IMO.
(Of course, if there's ever another event with such a generated string/message, it'll have to be an event, and that'll make the log stand out even more for no good reason...)
I just think making it an event was a good thing, but I don't feel very strongly about this, so I guess you could just revert to a separate log callback -- though I'm curious what's the main motivation for favoring this solution? Avoiding the malloc/free calls (A buffer could also be used/printed into then, leaving the malloc only for extra-long messages) ?
Incorporating log messages into the event callback also silenced log messages when --print is used, including debug messages, which needs
I don't understand. The commit that introduced log messages as event, that Allan will likely revert, shouldn't have changed anything in how/when said messages are printed, only how they are sent to the front-end (as in, which callback function is used). Also, I just ran `pacman --debug -S --print pacman` and I see debug messages just fine. So I'm not sure what you mean, could you tell me how to reproduce it?
to be fixed. So, in order to keep it in the event callback you would have to:
* create a special case for the log event in the callback when --print is used
As said earlier, I don't see why a special case is needed/what was broken here.
* malloc the formatted string in the back-end solely for the purpose of passing it to the front-end even if it will be ignored or could have been printed directly without the malloc
True (although it could be printed to a buffer, but yes)
* malloc a second copy of the string in the front-end whenever it's in the middle of a progress bar
True, but in that case pacman always mallocs its own copy of the message anyways, so nothing new here.
I don't think keeping log messages in the event callback is worth the hassle.
apg