On Monday 03 of September 2012 14:15:34 Denys Vlasenko wrote:
> On 08/16/2012 03:11 PM, Jakub Filak wrote:
> > - the mapping was obtained from this link:
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=kern
> > el/panic.c
> > 
> > Signed-off-by: Jakub Filak <[email protected]>
> > ---
> > 
> >  src/include/libabrt.h        |  2 ++
> >  src/lib/kernel.c             | 74
> >  +++++++++++++++++++++-----------------------
> >  src/plugins/abrt-dump-oops.c | 24 ++++++--------
> >  3 files changed, 48 insertions(+), 52 deletions(-)
> > 
> > diff --git a/src/include/libabrt.h b/src/include/libabrt.h
> > index 48d2ac3..22aed28 100644
> > --- a/src/include/libabrt.h
> > +++ b/src/include/libabrt.h
> > @@ -79,6 +79,8 @@ int daemon_is_ok();
> > 
> >  char *koops_extract_version(const char *line);
> >  #define kernel_tainted_short abrt_kernel_tainted_short
> >  char *kernel_tainted_short(const char *kernel_bt);
> > 
> > +#define kernel_tainted_long abrt_kernel_tainted_long
> > +GList *kernel_tainted_long(const char *tainted_short);
> > 
> >  #define koops_hash_str abrt_koops_hash_str
> >  void koops_hash_str(char hash_str[SHA1_RESULT_LEN*2 + 1], char *oops_buf,
> >  const char *oops_ptr); #define koops_extract_oopses
> >  abrt_koops_extract_oopses
> > 
> > diff --git a/src/lib/kernel.c b/src/lib/kernel.c
> > index 9655296..9ffcb72 100644
> > --- a/src/lib/kernel.c
> > +++ b/src/lib/kernel.c
> > @@ -555,56 +555,54 @@ char *kernel_tainted_short(const char *kernel_bt)
> > 
> >      return tnt;
> >  
> >  }
> > 
> > -#if 0 /* unused */
> > 
> >  static const char *const tnts_long[] = {
> > 
> > -    "Proprietary module has been loaded.",
> > -    "Module has been forcibly loaded.",
> > -    "SMP with CPUs not designed for SMP.",
> > -    "User forced a module unload.",
> > -    "System experienced a machine check exception.",
> > -    "System has hit bad_page.",
> > -    "Userspace-defined naughtiness.",
> > -    "Kernel has oopsed before.",
> > -    "ACPI table overridden.",
> > -    "Taint on warning.",
> > -    "Modules from drivers/staging are loaded.",
> > -    "Working around severe firmware bug.",
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    NULL,
> > -    "Hardware is unsupported.",
> > -    "Tech_preview",
> > +    /* A */ "ACPI table overridden.",
> > +    /* B */ "System has hit bad_page.",
> > +    /* C */ "Modules from drivers/staging are loaded.",
> > +    /* D */ "Kernel has oopsed before",
> > +    /* E */ NULL,
> > +    /* F */ "Module has been forcibly loaded.",
> > +    /* G */ "Proprietary module has not been loaded.",
> > +    /* H */ NULL,
> > +    /* I */ "Working around severe firmware bug.",
> > +    /* J */ NULL,
> > +    /* K */ NULL,
> > +    /* L */ NULL,
> > +    /* M */ "System experienced a machine check exception.",
> > +    /* N */ NULL,
> > +    /* O */ "Out-of-tree module has been loaded.",
> > +    /* P */ "Proprietary module has been loaded.",
> > +    /* Q */ NULL,
> > +    /* R */ "User forced a module unload.",
> > +    /* S */ "SMP with CPUs not designed for SMP.",
> > +    /* T */ NULL,
> > +    /* U */ "Userspace-defined naughtiness.",
> > +    /* V */ NULL,
> > +    /* W */ "Taint on warning.",
> > +    /* X */ NULL,
> > +    /* Y */ NULL,
> > +    /* Z */ NULL,
> > 
> >  };
> > 
> > -GList *kernel_tainted_long(unsigned tainted)
> > +GList *kernel_tainted_long(const char *tainted_short)
> > 
> >  {
> >  
> >      int i = 0;
> >      GList *tnt = NULL;
> > 
> > -    while (tainted)
> > +    while (tainted_short[0] != '\0')
> > 
> >      {
> > 
> > -        if ((0x1 & tainted) && tnts_long[i])
> > -            tnt = g_list_append(tnt, xstrdup(tnts_long[i]));
> > +        const int tnt_index = tainted_short[0] - 'A';
> > +        if (tnt_index >= 0 && tnt_index <= 'Z' - 'A')
> > +        {
> > +            const char *const txt = tnts_long[tnt_index];
> > +            if (txt)
> > +                tnt = g_list_append(tnt, (gpointer)txt);
> > +        }
> > 
> >          ++i;
> > 
> > -        tainted >>= 1;
> > +        ++tainted_short;
> > 
> >      }
> >      
> >      return tnt;
> >  
> >  }
> > 
> > -#endif
> > 
> > diff --git a/src/plugins/abrt-dump-oops.c b/src/plugins/abrt-dump-oops.c
> > index 29e74c5..016166c 100644
> > --- a/src/plugins/abrt-dump-oops.c
> > +++ b/src/plugins/abrt-dump-oops.c
> > @@ -128,6 +128,16 @@ static unsigned save_oops_to_dump_dir(GList
> > *oops_list, unsigned oops_cnt)> 
> >              {
> >              
> >                  VERB1 log("Kernel is tainted '%s'", tainted_short);
> >                  dd_save_text(dd, FILENAME_TAINTED_SHORT, tainted_short);
> > 
> > +
> > +                GList *tainted_long = kernel_tainted_long(tainted_short);
> > +                struct strbuf *tnt_long = strbuf_new();
> > +                for (GList *li = tainted_long; li; li = li->next)
> > +                    strbuf_append_strf(tnt_long, "%s\n", (char*)
> > li->data); +
> > +                dd_save_text(dd, FILENAME_TAINTED_LONG, tnt_long->buf);
> > +                strbuf_free(tnt_long);
> > +                g_list_free(tainted_long);
> > +
> > 
> >                  const char *fmt = _("A kernel problem occurred, but your
> >                  kernel has been "
> >                  
> >                               "tainted (flags:%s). Kernel maintainers are
> >                               unable to "
> >                               "diagnose tainted reports.");
> > 
> > @@ -141,20 +151,6 @@ static unsigned save_oops_to_dump_dir(GList
> > *oops_list, unsigned oops_cnt)> 
> >  // kernel oops 1st line may look quite puzzling otherwise...
> >  
> >              strchrnul(second_line, '\n')[0] = '\0';
> >              dd_save_text(dd, FILENAME_REASON, second_line);
> > 
> > -
> > -/*
> > -            GList *tainted_long = kernel_tainted_long(tainted);
> > -
> > -            struct strbuf *tnt_long = strbuf_new();
> > -            for (GList *li = tainted_long; li; li = li->next)
> > -                strbuf_append_strf(tnt_long, "%s\n", (char*) li->data);
> > -
> > -            dd_save_text(dd, FILENAME_TAINTED, tainted_str);
> > -            dd_save_text(dd, FILENAME_TAINTED_SHORT, tainted_short);
> > -            dd_save_text(dd, FILENAME_TAINTED_LONG, tnt_long->buf);
> > -            strbuf_free(tnt_long);
> > -            list_free_with_free(tainted_long);
> > -*/
> > 
> >              dd_close(dd);
> >          
> >          }
> >          else
> 
> I would make kernel_tainted_long() function static and put it
> into single file which uses it. *If* we will need it elsewhere
> too, then we can move it out and expose in the API.

kernel_tainted_long() function was always placed in the abrt/lib/kernel.c file 
and I don't think that it is a good idea to move it from its origin somewhere 
else.

> 
> I would just generate a string at once instead
> of going through intermediate GList.

It make sense and I'll do it. (I simply extended a body of the function 
because I always try to achieve my goals with minimal number of changes.)

Reply via email to