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 <jfilak@redhat.com>

> > ---

> >

> > 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.)