[pacman-dev] [PATCH v2 2/2] Update the question callback

[Olivier Brunel]

On 07/02/14 01:10, Andrew Gregory wrote:
> On 07/02/14 at 01:02am, Olivier Brunel wrote:
>> 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?
> 
>  void cb_event(alpm_event_t *event)
>  {
>  	if(config->print) {
>  		return;
>  	}
> 
> The only messages you see are from pacman not alpm.

Oh right, got it; thanks. Yeah the case of ALPM_EVENT_LOG should have
been handled before that test, indeed.

> 
> apg
>