From: Daniel H. <dan...@li...> - 2008-04-09 13:07:02
|
Maynard Johnson wrote: > Maynard Johnson wrote: >> Here is the patch to separate the conversion functionality in its own >> module. > Just a couple minor comments . . . otherwise looks fine to me. > > -Maynard > Hi Maynard, thanks for your comments. Kind regards, Daniel >> >> That is useful to separate the code handling potentially insecure data >> from the rest of the code. >> >> Kind regards, >> Daniel >> >> > [snip] >> + >> diff -prauN -X ../workspaces/eclipse/excludelist.txt >> ../workspaces/eclipse/oprofile/opjitconv/conversion.h >> oprofile_conv_extracted/opjitconv/conversion.h >> --- ../workspaces/eclipse/oprofile/opjitconv/conversion.h >> 1970-01-01 01:00:00.000000000 +0100 >> +++ oprofile_conv_extracted/opjitconv/conversion.h 2008-03-18 > Why not put this prototype into opjitconv.h instead of creating another > header file? Right. I have seen that such prototypes are declared in opjitconv.h as you mentioned. >> 11:28:39.000000000 +0100 >> @@ -0,0 0+1,9 @@ >> +#ifndef CONVERSION_H_ >> +#define CONVERSION_H_ >> + >> +#include "opjitconv.h" >> + >> +int op_jit_convert(struct op_jitdump_info file_info, char const * >> elffile, >> + unsigned long long start_time, unsigned long long >> end_time); >> + >> +#endif /*CONVERSION_H_*/ >> diff -prauN -X ../workspaces/eclipse/excludelist.txt >> ../workspaces/eclipse/oprofile/opjitconv/Makefile.am >> oprofile_conv_extracted/opjitconv/Makefile.am >> --- ../workspaces/eclipse/oprofile/opjitconv/Makefile.am 2007-11-27 >> 16:14:01.000000000 +0100 >> +++ oprofile_conv_extracted/opjitconv/Makefile.am 2008-03-18 >> 11:12:36.000000000 +0100 >> @@ -13,6 +13,7 @@ opjitconv_LDADD = $(needed_libs) >> opjitconv_SOURCES = \ >> opjitconv.c \ >> opjitconv.h \ >> + conversion.c \ >> parse_dump.c \ >> jitsymbol.c \ >> create_bfd.c \ >> diff -prauN -X ../workspaces/eclipse/excludelist.txt >> ../workspaces/eclipse/oprofile/opjitconv/opjitconv.c >> oprofile_conv_extracted/opjitconv/opjitconv.c >> --- ../workspaces/eclipse/oprofile/opjitconv/opjitconv.c 2008-04-03 >> 14:56:00.000000000 +0200 >> +++ oprofile_conv_extracted/opjitconv/opjitconv.c 2008-04-03 >> 11:04:02.000000000 +0200 >> @@ -14,13 +14,11 @@ >> */ > [snip] >> + + /* Check if the dump file is a symbolic link. >> + * We should not trust symbolic links because we only produce >> normal dump >> + * files (no links). >> + */ >> + if ((lstat(dmp_pathname, &file_stat) == -1) || >> + (S_ISLNK(file_stat.st_mode))) { >> + printf("opjitconv: dumpfile path is corrupt (symbolic links >> not allowed).\n"); > If lstat returns -1, then the printf message will be misleading. My first error message was a little bit different. I changed it at the end regarding to symbolic links. But I forgot the "lstat() == -1" condition. Thanks for this correction. >> + rc = OP_JIT_CONV_FAIL; >> + goto out; >> + } >> + if (dumpfilename) { >> char const * dot_dump = rindex(++dumpfilename, '.'); > [snip] > |