It seems that _COMM is limited to 16 characters, which you can see by listing /proc/<pid>/comm (which is presumably what journald does to fill the _COMM field).

It is probably easiest to fetch and parse _CMDLINE instead.

Created attachment 296248 [details] [review]
use "kernel" as the name of kernel-generated log messages

kernel-generated log messages don't have _COMM and _CMDLINE. So it's not useful to parse _CMDLINE, we can name these log messages as "kernel".

Review of attachment 296248 [details] [review]:

Any message can have a _COMM that is empty, as argv can be changed by the application at runtime (or the process can simply write to /proc/[pid]/comm), so this is not a correct fix. Also, it is not a fix for this bug, which is about extending the application name for each message to be longer than 16 characters, if available. Kernel messages should be checked with the _TRANSPORT field.

Created attachment 296314 [details] [review]
parse _CMDLINE to name log messages

I didn't fully get the main idea of the bug. Sorry for that.
Here's the patch for parsing _CMDLINE to name log messages. I don't have enough kinds of log messages to test it. Hope you can help test this patch.

Review of attachment 296314 [details] [review]:

This lead to reproducible crashes for me, when selecting the details view with certain entries, so will need a bit more work.

::: src/gl-eventviewrow.c
@@ +108,2 @@
 static void
+gl_event_view_row_parse_cmdline (GlEventViewRow *row)

This should accept a "gchar *" (or even a "const gchar *") rather than a GlEventViewRow, and return a gchar *. This would make it much easier to test, as the test would not have to contruct a complete GlEventViewRow just to test the cmdline parsing.

Created attachment 296825 [details] [review]
parse _CMDLINE to name log messages

I fixed the problem, and this patch should work.

Review of attachment 296825 [details] [review]:

::: src/gl-eventviewrow.c
@@ +119,3 @@
+    {
+        str = strtok (str2, "/");
+        while ((str = strtok (NULL, "/")) != NULL)

Leave a blank line before the while.

@@ +126,3 @@
+    else
+    {
+		rtn = str1;

Odd indentation here.

@@ +128,3 @@
+		rtn = str1;
+        str = strtok (str1, "/");
+        while ((str = strtok (NULL, "/")) != NULL)

Leave a blank line before the while.

@@ +162,3 @@
     gtk_container_add (GTK_CONTAINER (row), grid);
 
+    result->cmdline = gl_event_view_row_parse_cmdline (result->cmdline);

Now that result->cmdline has been changed, it is not possible to call g_free() on it (as it does not necessarily point to the same location). You will probably need to accept a const gchar * in _parse_cmdline(), and return a duplicated string, taking care to free result->cmdline as necessary.

::: src/gl-eventviewrow.h
@@ +59,3 @@
 GtkWidget * gl_event_view_row_new (GlJournalResult *result, GlEventViewRowStyle style, GlUtilClockFormat clock_format);
 GlJournalResult * gl_event_view_row_get_result (GlEventViewRow *row);
+gchar * gl_event_view_row_parse_cmdline (gchar *cmdline);

Why did you add this function to the header? It is not used in any other file, and so should be static.

::: src/gl-journal.c
@@ +293,3 @@
     }
 
+	result->cmdline = gl_journal_get_data (self, "_CMDLINE", &error);

Odd indentation.

Created attachment 296874 [details] [review]
parse _CMDLINE to name log messages

Fixed a few things. And here is the patch.

Review of attachment 296874 [details] [review]:

This doesn't fix the issue that I mentioned before. Make gl_event_view_row_parse_cmdline() accept a "const gchar *", and that should solve the problem.

If the argument of _parse_cmdline() is const type, there will be a warning that g_strtok ignores const while compiling.

Correct. You need to adjust the code until there is not a warning.

Created attachment 296877 [details] [review]
parse _CMDLINE to name log messages

This should work.

Comment on attachment 296877 [details] [review]
parse _CMDLINE to name log messages

@@ -130,8 +163,9 @@ gl_event_view_row_create_cmdline (GlEventViewRow *row)
     gtk_grid_set_column_spacing (GTK_GRID (grid), 6);
     gtk_container_add (GTK_CONTAINER (row), grid);

+    result->cmdline = gl_event_view_row_parse_cmdline (result->cmdline);
     markup = g_markup_printf_escaped ("<b>%s</b>",
-                                      result->comm ? result->comm : "");

This is a memory leak. You need to be careful to avoid them.

Parsing command lines is hard. Can't we just use _EXE and call g_path_get_basename() on it?

The problem with _EXE is that it points to the interpreter for Python (and bash, and…) scripts.

Hm, good point. But I don't see a way around that. There's no way to tell which argument is the name of the script. The attached patch guesses it's always the second argument, but only if it starts with '/'. Sometimes that's not true, and sometimes there's no script name at all. For example, I use this all the time:

  python3 -m http.server

I think I'd want the full command line in that case. Maybe we should just always show that?

Also, are we going to need this at all once bug #727895 is fixed? The new design doesn't seem to show program name at all.

The details view (accessed by clicking on a row) shows an icon for the application, if a (admittedly, very crude) lookup against COMM matches a desktop file. Where a desktop file is not available, the application name is shown, if available. The examples given in this report could be solved by querying the systemd service name.

It is really hard to unambiguously match a log message with an "application" at the moment, but if we restrict the definition of an application to something well-defined then we should be fine. It is easiest to request that applications inject extra metadata into the journal for the purposes of matching an application with a desktop file, or maybe this is something that gnome-shell or the application sandbox should be doing?

I think using SYSLOG_IDENTIFIER instead of _COMM field as the title in detail view is a better option(if no icon is shown).

SYSLOG_IDENTIFIER is roughly the value of _CMDLINE, so not appropriate as a replacement for COMM in several cases. Plus, we want the trusted fields, not the user fields.

(In reply to David King from comment #19)
> SYSLOG_IDENTIFIER is roughly the value of _CMDLINE, so not appropriate as a
> replacement for COMM in several cases. Plus, we want the trusted fields, not
> the user fields.

I think SYSLOG_IDENTIFIER is quite different with _CMDLINE. For example, I got such a log entry in my system:

Fri 2016-02-19 16:53:36.103845 CST [s=211df74eb4364b3ca31d9a24873f4a81;i=1dc9c;b=f5bafa6ed3644776bcfc5a1c9774b4b7;m=6f52cb379;t=52c1b9e5bb9a5;x=9d56721531592272]
    _TRANSPORT=stdout
    _UID=1000
    _GID=100
    _CAP_EFFECTIVE=0
    _AUDIT_SESSION=1
    _AUDIT_LOGINUID=1000
    _SYSTEMD_CGROUP=/user.slice/user-1000.slice/session-1.scope
    _SYSTEMD_SESSION=1
    _SYSTEMD_OWNER_UID=1000
    _SYSTEMD_UNIT=session-1.scope
    _SYSTEMD_SLICE=user-1000.slice
    _MACHINE_ID=89c660865c00403a9bacef32b6828556
    _HOSTNAME=sckang
    _COMM=dbus-daemon
    _EXE=/bin/dbus-daemon
    _CMDLINE=/bin/dbus-daemon --fork --print-pid 5 --print-address 7 --session
    PRIORITY=4
    SYSLOG_IDENTIFIER=org.gnome.Contacts.SearchProvider
    _PID=1760
    _BOOT_ID=f5bafa6ed3644776bcfc5a1c9774b4b7
    MESSAGE=(gnome-contacts-search-provider:17557): Gtk-CRITICAL **: gtk_list_store_insert_before: assertion 'iter_is_valid (sibling, list_store)' failed

SYSLOG_IDENTIFIER provides a more accurate information of the log entry than _COMM and _CMDLINE. Yep, SYSLOG_IDENTIFIER is not a trusted field. :-(

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-logs/issues/7.