This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.
Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Tue, Jan 25, 2005 at 02:15:01PM -0800, Ulrich Drepper wrote: > Jakub Jelinek wrote: > >if the string passed to addseverity is freed by the caller, > >we still reference freed up memory, or if the string passed to addseverity > >is overwritten, we use whatever it was overwritten to. > > I've never seen anything to the contrary that this is acceptable. The > function is not part of any spec and when I wrote this code this seemed > to be what other implementations did. What program has problems with > the current code? Ok, it seems Solaris addseverity is grossly misdesigned - works ala putenv, i.e. if you modify the string passed to it, it will change fmtmsg behaviour and as glibc has this for compatibility only, let's be compatible. I hope it is ok if we leak a few strings at exit time if SEV_LEVEL is set in environment and the app ever calls fmtmsg - it wouldn't be hard to handle that, but would mean using more memory and/or instructions for this obscure interface just to shut up mtrace/valgrind. (That leak is btw not introduced by this patch, has been there like forever). 2005-01-25 Jakub Jelinek <jakub@redhat.com> * stdlib/fmtmsg.c (addseverity): Remove new_string variable. (free_mem): Don't free string. * stdlib/tst-fmtmsg.c: Include string.h. (main): Add some more tests. --- libc/stdlib/tst-fmtmsg.c.jj 2005-01-19 14:13:03.000000000 +0100 +++ libc/stdlib/tst-fmtmsg.c 2005-01-25 23:48:12.221066369 +0100 @@ -1,6 +1,7 @@ #include <fmtmsg.h> #include <mcheck.h> #include <stdio.h> +#include <string.h> #define MM_TEST 10 @@ -12,11 +13,13 @@ main (void) mtrace (); - if (addseverity (MM_TEST, "TEST") != MM_OK) + char TEST[] = "ABCD"; + if (addseverity (MM_TEST, TEST) != MM_OK) { puts ("addseverity failed"); result = 1; } + strcpy (TEST, "TEST"); if (fmtmsg (MM_PRINT, "GLIBC:tst-fmtmsg", MM_HALT, "halt", "should print message for MM_HALT", "GLIBC:tst-fmtmsg:1") @@ -54,5 +57,25 @@ main (void) result = 1; } + if (addseverity (MM_TEST, NULL) != MM_NOTOK) + { + puts ("third addseverity unexpectedly succeeded"); + result = 1; + } + + char *p = strdup ("TEST2"); + if (addseverity (MM_TEST, p) != MM_OK) + { + puts ("fourth addseverity failed"); + result = 1; + } + if (addseverity (MM_TEST, "TEST3") != MM_OK) + { + puts ("fifth addseverity failed"); + result = 1; + } + + free (p); + return result; } --- libc/stdlib/fmtmsg.c.jj 2005-01-25 22:30:23.000000000 +0100 +++ libc/stdlib/fmtmsg.c 2005-01-25 23:47:29.079775049 +0100 @@ -316,7 +316,7 @@ internal_addseverity (int severity, cons int result = MM_OK; /* First see if there is already a record for the severity level. */ - for (runp = severity_list, lastp = NULL; runp != NULL; runp = runp-> next) + for (runp = severity_list, lastp = NULL; runp != NULL; runp = runp->next) if (runp->severity == severity) break; else @@ -364,34 +364,17 @@ int addseverity (int severity, const char *string) { int result; - const char *new_string; /* Prevent illegal SEVERITY values. */ if (severity <= MM_INFO) return MM_NOTOK; - if (string == NULL) - /* We want to remove the severity class. */ - new_string = NULL; - else - { - new_string = __strdup (string); - - if (new_string == NULL) - /* Allocation failed or illegal value. */ - return MM_NOTOK; - } - /* Protect the global data. */ __libc_lock_lock (lock); /* Do the real work. */ result = internal_addseverity (severity, string); - if (result != MM_OK) - /* Free the allocated string. */ - free ((char *) new_string); - /* Release the lock. */ __libc_lock_unlock (lock); @@ -408,7 +391,6 @@ libc_freeres_fn (free_mem) { /* This is data we have to release. */ struct severity_info *here = runp; - free ((char *) runp->string); runp = runp->next; free (here); } Jakub
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |