The shortlog should mention G_GNUC_PRINTF rather than format(printf) On Tue, Feb 19, 2019 at 03:00:09PM +0000, Daniel P. Berrangé wrote: > This allows the compiler to validate the format string and args passed > to the function. > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > --- > src/virt-viewer-app.c | 38 +++++++++++++++++++++++--------------- > src/virt-viewer-notebook.c | 4 ++-- > 2 files changed, 25 insertions(+), 17 deletions(-) > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c > index 2592484..15e7b3d 100644 > --- a/src/virt-viewer-app.c > +++ b/src/virt-viewer-app.c > @@ -197,19 +197,16 @@ virt_viewer_app_set_debug(gboolean debug) > doDebug = debug; > } > > -static GtkWidget* > -virt_viewer_app_make_message_dialog(VirtViewerApp *self, > - const char *fmt, ...) > +G_GNUC_PRINTF(2, 0) static GtkWidget* > +virt_viewer_app_make_message_dialogv(VirtViewerApp *self, > + const char *fmt, va_list vargs) Is the addition of this helper required? I'd put G_GNUC_PRINTF after the declaration: https://developer.gnome.org/glib/unstable/glib-Miscellaneous-Macros.html#G-GNUC-PRINTF:CAPS « Place the attribute after the function declaration, just before the semicolon. » We don't have a semi-colon here, but at the end of the declaration seems more consistent with this documentation. > { > g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), NULL); > GtkWindow *window = GTK_WINDOW(virt_viewer_window_get_window(self->priv->main_window)); > GtkWidget *dialog; > char *msg; > - va_list vargs; > > - va_start(vargs, fmt); > msg = g_strdup_vprintf(fmt, vargs); > - va_end(vargs); > > dialog = gtk_message_dialog_new(window, > GTK_DIALOG_MODAL | > @@ -224,23 +221,34 @@ virt_viewer_app_make_message_dialog(VirtViewerApp *self, > return dialog; > } > > -void > +G_GNUC_PRINTF(2, 3) static GtkWidget* > +virt_viewer_app_make_message_dialog(VirtViewerApp *self, > + const char *fmt, ...) > +{ > + g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), NULL); > + GtkWidget *dialog; > + va_list vargs; > + > + va_start(vargs, fmt); > + dialog = virt_viewer_app_make_message_dialogv(self, fmt, vargs); > + va_end(vargs); > + > + return dialog; > +} > + > +G_GNUC_PRINTF(2, 3) void > virt_viewer_app_simple_message_dialog(VirtViewerApp *self, > const char *fmt, ...) This should be done in the .h file, and same comment about putting this after the declaration. Same comments throughout the rest of this file. Christophe > { > GtkWidget *dialog; > - char *msg; > va_list vargs; > > va_start(vargs, fmt); > - msg = g_strdup_vprintf(fmt, vargs); > + dialog = virt_viewer_app_make_message_dialogv(self, fmt, vargs); > va_end(vargs); > > - dialog = virt_viewer_app_make_message_dialog(self, msg); > gtk_dialog_run(GTK_DIALOG(dialog)); > gtk_widget_destroy(dialog); > - > - g_free(msg); > } > > static void > @@ -668,7 +676,7 @@ virt_viewer_app_open_unix_sock(const char *unixsock, GError **error) > > #endif /* defined(HAVE_SOCKETPAIR) && defined(HAVE_FORK) */ > > -void > +G_GNUC_PRINTF(2, 3) void > virt_viewer_app_trace(VirtViewerApp *self, > const char *fmt, ...) > { > @@ -1871,7 +1879,7 @@ virt_viewer_app_on_application_startup(GApplication *app) > > if (!virt_viewer_app_start(self, &error)) { > if (error && !g_error_matches(error, VIRT_VIEWER_ERROR, VIRT_VIEWER_ERROR_CANCELLED)) > - virt_viewer_app_simple_message_dialog(self, error->message); > + virt_viewer_app_simple_message_dialog(self, "%s", error->message); > > g_clear_error(&error); > g_application_quit(app); > @@ -2477,7 +2485,7 @@ show_status_cb(gpointer value, > virt_viewer_notebook_show_status(nb, text); > } > > -void > +G_GNUC_PRINTF(2, 3) void > virt_viewer_app_show_status(VirtViewerApp *self, const gchar *fmt, ...) > { > va_list args; > diff --git a/src/virt-viewer-notebook.c b/src/virt-viewer-notebook.c > index 3a74e9f..310ea12 100644 > --- a/src/virt-viewer-notebook.c > +++ b/src/virt-viewer-notebook.c > @@ -82,7 +82,7 @@ virt_viewer_notebook_init (VirtViewerNotebook *self) > gtk_notebook_append_page(GTK_NOTEBOOK(self), priv->status, NULL); > } > > -void > +G_GNUC_PRINTF(2, 0) void > virt_viewer_notebook_show_status_va(VirtViewerNotebook *self, const gchar *fmt, va_list args) > { > VirtViewerNotebookPrivate *priv; > @@ -99,7 +99,7 @@ virt_viewer_notebook_show_status_va(VirtViewerNotebook *self, const gchar *fmt, > g_free(text); > } > > -void > +G_GNUC_PRINTF(2, 3) void > virt_viewer_notebook_show_status(VirtViewerNotebook *self, const gchar *fmt, ...) > { > va_list args; > -- > 2.20.1 > > _______________________________________________ > virt-tools-list mailing list > virt-tools-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/virt-tools-list
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
