From: Rob B. <ro...@o-...> - 2007-02-20 10:22:50
Attachments:
oparchive-debug-dir.patch
oparchive-list-files.patch
|
Attached are a couple of patches that we are using against our OProfile tree to improve oparchive. 1.) The first fixes a bug in oparchive that results in the the debug data overwriting the executables themselves. The patch fixes this by always putting the debug data into a .debug subdirectory. 'opreport' will find it there so it doesn't really matter that it might have originally been in /usr/lib/debug. 2.) The second adds a --list-files/-l option to oparchive that simply lists the files that would be copied rather than copying them. We'd love to see these patches integrated so welcome feedback. Cheers, Rob |
From: John L. <le...@mo...> - 2007-02-20 12:55:58
|
On Tue, Feb 20, 2007 at 10:22:21AM +0000, Rob Bradford wrote: > Attached are a couple of patches that we are using against our OProfile > tree to improve oparchive. Thanks. Will, can you review and apply? regards john |
From: William C. <wc...@re...> - 2007-02-20 14:40:06
|
John Levon wrote: > On Tue, Feb 20, 2007 at 10:22:21AM +0000, Rob Bradford wrote: > >> Attached are a couple of patches that we are using against our OProfile >> tree to improve oparchive. > > Thanks. Will, can you review and apply? > > regards > john Okay, I will take a look at the patch. -Will |
From: William C. <wc...@re...> - 2007-02-21 19:24:07
|
Rob Bradford wrote: > Attached are a couple of patches that we are using against our OProfile > tree to improve oparchive. > > 1.) The first fixes a bug in oparchive that results in the the debug > data overwriting the executables themselves. The patch fixes this by > always putting the debug data into a .debug subdirectory. 'opreport' > will find it there so it doesn't really matter that it might have > originally been in /usr/lib/debug. > > 2.) The second adds a --list-files/-l option to oparchive that simply > lists the files that would be copied rather than copying them. > > We'd love to see these patches integrated so welcome feedback. > > Cheers, > > Rob Hi, The patches work as described. Made some minor formatting fixes and checked them into the CVS. -Will |
From: John L. <le...@mo...> - 2007-02-21 19:24:20
|
On Tue, Feb 20, 2007 at 10:22:21AM +0000, Rob Bradford wrote: > 2.) The second adds a --list-files/-l option to oparchive that simply > lists the files that would be copied rather than copying them. Will, the man pages and XML doc needs updating too for this. regards, john |
From: William C. <wc...@re...> - 2007-02-21 19:47:10
|
Rob Bradford wrote: > Attached are a couple of patches that we are using against our OProfile > tree to improve oparchive. > > 1.) The first fixes a bug in oparchive that results in the the debug > data overwriting the executables themselves. The patch fixes this by > always putting the debug data into a .debug subdirectory. 'opreport' > will find it there so it doesn't really matter that it might have > originally been in /usr/lib/debug. > > 2.) The second adds a --list-files/-l option to oparchive that simply > lists the files that would be copied rather than copying them. > > We'd love to see these patches integrated so welcome feedback. > > Cheers, > > Rob Hi Rob, The first patch does prevent a possible overwrite of executable file. Are there concrete examples file name is being used for both the executable and the debuginfo file? That case is the second patch patch useful? Note that it doesn't totally disable creation of file system. The first patch may create directories even if the -l option is being used. Documentation for the -l options for -Will |
From: Rob B. <ro...@o-...> - 2007-02-22 12:39:39
|
On Wed, 2007-02-21 at 14:46 -0500, William Cohen wrote: > Rob Bradford wrote: > The first patch does prevent a possible overwrite of executable file. Are there > concrete examples file name is being used for both the executable and the > debuginfo file? OpenEmbedded puts the debug data for, e.g. busybox into /bin/.debug/busybox. So in this case the debug data would be copied over the binary. T > That case is the second patch patch useful? Note that it doesn't totally disable > creation of file system. The first patch may create directories even if the -l > option is being used. Documentation for the -l options for It looks like your mail was cut-off here Uhm. That is a bug, I did only want the file names that would be copied to be printed. Nothing more. Cheers, Rob |
From: Rob B. <ro...@o-...> - 2007-02-22 15:49:40
|
On Thu, 2007-02-22 at 10:05 -0500, William Cohen wrote: > Rob Bradford wrote: > > On Wed, 2007-02-21 at 14:46 -0500, William Cohen wrote: > >> Rob Bradford wrote: > > > >> The first patch does prevent a possible overwrite of executable file. Are there > >> concrete examples file name is being used for both the executable and the > >> debuginfo file? > > > > OpenEmbedded puts the debug data for, e.g. busybox > > into /bin/.debug/busybox. So in this case the debug data would be copied > > over the binary. T > > > > Okay, I suspected that it was due to a different distro. The motivation for the > patch makes sense. > > The preceeding comment in the code about where the debuginfo is being placed is > going to need to be changed. Please could you be a little bit more specific. > Could the create_path function be used instead of mkdir function in the patch to > make the patch a little more compact? Could eliminate the dest_debug_dir > variable and separate check for the errno. Okay. > >> That case is the second patch patch useful? Note that it doesn't totally disable > >> creation of file system. The first patch may create directories even if the -l > >> option is being used. Documentation for the -l options for > > > > It looks like your mail was cut-off here > > > > Uhm. That is a bug, I did only want the file names that would be copied > > to be printed. Nothing more. > > The question is why do you want just a list of the files being gathered up? Is > there some other processing being done with that? We are using it for a simple server for getting the complete archive off an embedded target device for processing on a host. I thought it would be useful for other people who might want to do similar things/or as a dry run for testing. > Sorry for the cut-off in the email. Documentation for the -l option for > oparchive should be include in the patch. > > The -l will need to disable the create_path calls in the code. Okay. Cheers, Rob |
From: William C. <wc...@re...> - 2007-02-22 16:27:52
|
Rob Bradford wrote: > On Thu, 2007-02-22 at 10:05 -0500, William Cohen wrote: >> Rob Bradford wrote: >>> On Wed, 2007-02-21 at 14:46 -0500, William Cohen wrote: >>>> Rob Bradford wrote: >>>> The first patch does prevent a possible overwrite of executable file. Are there >>>> concrete examples file name is being used for both the executable and the >>>> debuginfo file? >>> OpenEmbedded puts the debug data for, e.g. busybox >>> into /bin/.debug/busybox. So in this case the debug data would be copied >>> over the binary. T >>> >> Okay, I suspected that it was due to a different distro. The motivation for the >> patch makes sense. >> >> The preceeding comment in the code about where the debuginfo is being placed is >> going to need to be changed. > > Please could you be a little bit more specific. Just before the code that copies the debuginfo file is the following comment: /* If there are any debuginfo files, copy them over. * Need to copy the debug info file in the same * directory as the executable. The /usr/lib/debug * search path is not going to work. */ It will need to be changed due to the patch. > >> Could the create_path function be used instead of mkdir function in the patch to >> make the patch a little more compact? Could eliminate the dest_debug_dir >> variable and separate check for the errno. > > Okay. > >>>> That case is the second patch patch useful? Note that it doesn't totally disable >>>> creation of file system. The first patch may create directories even if the -l >>>> option is being used. Documentation for the -l options for >>> It looks like your mail was cut-off here >>> >>> Uhm. That is a bug, I did only want the file names that would be copied >>> to be printed. Nothing more. >> The question is why do you want just a list of the files being gathered up? Is >> there some other processing being done with that? > > We are using it for a simple server for getting the complete archive off > an embedded target device for processing on a host. I thought it would > be useful for other people who might want to do similar things/or as a > dry run for testing. On the embedded platform you probably already disk image on the host system. You probably don't want to have embedded system package up the executables and debuginfo files. There might be a better solution than just printing out the file names being being archived. Want to merge the sample files from target and the disk image on the host (maybe with the debuginfo files installed). Could you think through this and generate a patch to make oprofile easier to use in this data analysis on host of target data. -Will |
From: William C. <wc...@re...> - 2007-02-22 15:05:57
|
Rob Bradford wrote: > On Wed, 2007-02-21 at 14:46 -0500, William Cohen wrote: >> Rob Bradford wrote: > >> The first patch does prevent a possible overwrite of executable file. Are there >> concrete examples file name is being used for both the executable and the >> debuginfo file? > > OpenEmbedded puts the debug data for, e.g. busybox > into /bin/.debug/busybox. So in this case the debug data would be copied > over the binary. T > Okay, I suspected that it was due to a different distro. The motivation for the patch makes sense. The preceeding comment in the code about where the debuginfo is being placed is going to need to be changed. Could the create_path function be used instead of mkdir function in the patch to make the patch a little more compact? Could eliminate the dest_debug_dir variable and separate check for the errno. >> That case is the second patch patch useful? Note that it doesn't totally disable >> creation of file system. The first patch may create directories even if the -l >> option is being used. Documentation for the -l options for > > It looks like your mail was cut-off here > > Uhm. That is a bug, I did only want the file names that would be copied > to be printed. Nothing more. The question is why do you want just a list of the files being gathered up? Is there some other processing being done with that? Sorry for the cut-off in the email. Documentation for the -l option for oparchive should be include in the patch. The -l will need to disable the create_path calls in the code. -Will |