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:
On 05/24/14 at 03:49pm, Olivier Brunel wrote: > Much like with events, instead of using a bunch of void* arguments for > all questions, we now send one pointer to an alpm_question_t struct. > This contains the type of question that was triggered. > > With this information, the pointer can then be typecasted to the > question-specific struct in order to get additional arguments. > > Signed-off-by: Olivier Brunel <jjk@jjacky.com> > --- > lib/libalpm/alpm.h | 92 +++++++++++++++++++++++++++++++++++++++-- > lib/libalpm/deps.c | 38 ++++++++++------- > lib/libalpm/handle.h | 4 +- > lib/libalpm/signing.c | 12 ++++-- > lib/libalpm/sync.c | 60 +++++++++++++++++---------- > src/pacman/callback.c | 110 +++++++++++++++++++++++++++----------------------- > src/pacman/callback.h | 3 +- > 7 files changed, 222 insertions(+), 97 deletions(-)
The generic to specific struct casts cause Clang to fail when pacman is configured with --enable-warningflags:
callback.c:377:44: error: cast from 'alpm_question_t *' (aka 'struct _alpm_question_t *') to 'alpm_question_install_ignorepkg_t *' (aka 'struct _alpm_question_install_ignorepkg_t *') increases required alignment from 4 to 8 [-Werror,-Wcast-align] ...*q = (alpm_question_install_ignorepkg_t *) question; ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
These are silenced.
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?
Anyway, possible ways to fix this:
1) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wformat-nonliteral" vprintf(event->fmt, event->args); +#pragma GCC diagnostic pop
This is crap because all clients that want to print a log message will need to wrap their calls like this.
2) Revert that change that made the logging an event.
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) ? -j
Allan