[PATCH] v4l2-tracer: Fix libv4l2tracer.so loader

[PATCH] v4l2-tracer: Fix libv4l2tracer.so loader

[Deborah Brouwer]

From: Nicolas Dufresne <nicolas.dufresne@collabora.com>

The code was still assuming libtools being use, so it didn't work
installed anymore. Also, it didn't work installed if the full
v4l2-tracer path was being passed.

Fix this by always trying next by libv4l2tracer.so loading (using stat()
to validate) and always fallback to the installed path otherwise.

Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
Hi - thanks to Nicolas for this patch.
I tested tracing and retracing on both installed and uninstalled
v4l2-tracer including with and without an absolute path and it
all works as expected.

Note that I've got three sets of v4l2-tracer patches outstanding now,
but they all still apply independently of each other:

Tuner patches:
https://lore.kernel.org/linux-media/cover.1684453027.git.deborah.brouwer@collabora.com/

Debug patches:
https://lore.kernel.org/linux-media/cover.1681245372.git.deborah.brouwer@collabora.com/

Thanks,
Deb

 utils/v4l2-tracer/v4l2-tracer.cpp | 33 +++++++++++++++++--------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/utils/v4l2-tracer/v4l2-tracer.cpp b/utils/v4l2-tracer/v4l2-tracer.cpp
index e3f002a9..7c3662be 100644
--- a/utils/v4l2-tracer/v4l2-tracer.cpp
+++ b/utils/v4l2-tracer/v4l2-tracer.cpp
@@ -5,6 +5,7 @@
 
 #include "retrace.h"
 #include <climits>
+#include <sys/stat.h>
 #include <sys/wait.h>
 #include <time.h>
 
@@ -295,24 +296,26 @@ int tracer(int argc, char *argv[], bool retrace)
 	fclose(trace_file);
 
 	/*
-	 * Preload the libv4l2tracer library. If the program is installed, load the library
-	 * from its installed location, otherwise load it locally. If it's loaded locally,
-	 * use ./configure --disable-dyn-libv4l.
+	 * Preload the libv4l2tracer library. The libv4l2tracer is looked up next to
+	 * the executable first in order to support uninstalled build.
 	 */
 	std::string libv4l2tracer_path;
 	std::string program = argv[0];
-	std::size_t idx = program.rfind("/v4l2-tracer");
-	if (idx != std::string::npos) {
-		libv4l2tracer_path = program.replace(program.begin() + idx + 1, program.end(), ".libs");
-		DIR *directory_pointer = opendir(libv4l2tracer_path.c_str());
-		if (directory_pointer == nullptr)
-			libv4l2tracer_path = program.replace(program.begin() + idx, program.end(), "./.libs");
-		else
-			closedir(directory_pointer);
-	} else {
-		libv4l2tracer_path = LIBTRACER_PATH;
-	}
-	libv4l2tracer_path += "/libv4l2tracer.so";
+	std::size_t idx = program.rfind("/");
+	struct stat sb;
+
+	if (idx == std::string::npos)
+		idx = 0;
+	else
+		idx++;
+
+	/* look for libv4l2tracer next to the executable */
+	libv4l2tracer_path = program.replace(program.begin() + idx, program.end(), "libv4l2tracer.so");
+
+	/* otherwise, use the installation path */
+	if (stat(libv4l2tracer_path.c_str(), &sb) == -1)
+		libv4l2tracer_path = std::string(LIBTRACER_PATH) + "/libv4l2tracer.so";
+
 	if (is_verbose())
 		fprintf(stderr, "Loading libv4l2tracer: %s\n", libv4l2tracer_path.c_str());
 	setenv("LD_PRELOAD", libv4l2tracer_path.c_str(), 0);
-- 
2.40.1

Re: [PATCH] v4l2-tracer: Fix libv4l2tracer.so loader

[Nicolas Dufresne]

Thanks Deborah,

Le mercredi 24 mai 2023 à 17:20 -0700, Deborah Brouwer a écrit :
> From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> The code was still assuming libtools being use, so it didn't work
> installed anymore. Also, it didn't work installed if the full
> v4l2-tracer path was being passed.
> 
> Fix this by always trying next by libv4l2tracer.so loading (using stat()
> to validate) and always fallback to the installed path otherwise.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

Mind adding you Singed-off-by ?

> ---
> Hi - thanks to Nicolas for this patch.
> I tested tracing and retracing on both installed and uninstalled
> v4l2-tracer including with and without an absolute path and it
> all works as expected.
> 
> Note that I've got three sets of v4l2-tracer patches outstanding now,
> but they all still apply independently of each other:
> 
> Tuner patches:
> https://lore.kernel.org/linux-media/cover.1684453027.git.deborah.brouwer@collabora.com/
> 
> Debug patches:
> https://lore.kernel.org/linux-media/cover.1681245372.git.deborah.brouwer@collabora.com/
> 
> Thanks,
> Deb
> 
>  utils/v4l2-tracer/v4l2-tracer.cpp | 33 +++++++++++++++++--------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/utils/v4l2-tracer/v4l2-tracer.cpp b/utils/v4l2-tracer/v4l2-tracer.cpp
> index e3f002a9..7c3662be 100644
> --- a/utils/v4l2-tracer/v4l2-tracer.cpp
> +++ b/utils/v4l2-tracer/v4l2-tracer.cpp
> @@ -5,6 +5,7 @@
>  
>  #include "retrace.h"
>  #include <climits>
> +#include <sys/stat.h>
>  #include <sys/wait.h>
>  #include <time.h>
>  
> @@ -295,24 +296,26 @@ int tracer(int argc, char *argv[], bool retrace)
>  	fclose(trace_file);
>  
>  	/*
> -	 * Preload the libv4l2tracer library. If the program is installed, load the library
> -	 * from its installed location, otherwise load it locally. If it's loaded locally,
> -	 * use ./configure --disable-dyn-libv4l.
> +	 * Preload the libv4l2tracer library. The libv4l2tracer is looked up next to
> +	 * the executable first in order to support uninstalled build.
>  	 */
>  	std::string libv4l2tracer_path;
>  	std::string program = argv[0];
> -	std::size_t idx = program.rfind("/v4l2-tracer");
> -	if (idx != std::string::npos) {
> -		libv4l2tracer_path = program.replace(program.begin() + idx + 1, program.end(), ".libs");
> -		DIR *directory_pointer = opendir(libv4l2tracer_path.c_str());
> -		if (directory_pointer == nullptr)
> -			libv4l2tracer_path = program.replace(program.begin() + idx, program.end(), "./.libs");
> -		else
> -			closedir(directory_pointer);
> -	} else {
> -		libv4l2tracer_path = LIBTRACER_PATH;
> -	}
> -	libv4l2tracer_path += "/libv4l2tracer.so";
> +	std::size_t idx = program.rfind("/");
> +	struct stat sb;
> +
> +	if (idx == std::string::npos)
> +		idx = 0;
> +	else
> +		idx++;
> +
> +	/* look for libv4l2tracer next to the executable */
> +	libv4l2tracer_path = program.replace(program.begin() + idx, program.end(), "libv4l2tracer.so");
> +
> +	/* otherwise, use the installation path */
> +	if (stat(libv4l2tracer_path.c_str(), &sb) == -1)
> +		libv4l2tracer_path = std::string(LIBTRACER_PATH) + "/libv4l2tracer.so";
> +
>  	if (is_verbose())
>  		fprintf(stderr, "Loading libv4l2tracer: %s\n", libv4l2tracer_path.c_str());
>  	setenv("LD_PRELOAD", libv4l2tracer_path.c_str(), 0);

Re: [PATCH] v4l2-tracer: Fix libv4l2tracer.so loader

[Deborah Brouwer]

On Thu, May 25, 2023 at 12:45:28PM -0400, Nicolas Dufresne wrote:
> Thanks Deborah,
> 
> Le mercredi 24 mai 2023 à 17:20 -0700, Deborah Brouwer a écrit :
> > From: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > The code was still assuming libtools being use, so it didn't work
> > installed anymore. Also, it didn't work installed if the full
> > v4l2-tracer path was being passed.
> > 
> > Fix this by always trying next by libv4l2tracer.so loading (using stat()
> > to validate) and always fallback to the installed path otherwise.
> > 
> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> Mind adding you Singed-off-by ?

Actually Hans already applied your patch (thanks!) but will do it next
time.

> 
> > ---
> > Hi - thanks to Nicolas for this patch.
> > I tested tracing and retracing on both installed and uninstalled
> > v4l2-tracer including with and without an absolute path and it
> > all works as expected.
> > 
> > Note that I've got three sets of v4l2-tracer patches outstanding now,
> > but they all still apply independently of each other:
> > 
> > Tuner patches:
> > https://lore.kernel.org/linux-media/cover.1684453027.git.deborah.brouwer@collabora.com/
> > 
> > Debug patches:
> > https://lore.kernel.org/linux-media/cover.1681245372.git.deborah.brouwer@collabora.com/
> > 
> > Thanks,
> > Deb
> > 
> >  utils/v4l2-tracer/v4l2-tracer.cpp | 33 +++++++++++++++++--------------
> >  1 file changed, 18 insertions(+), 15 deletions(-)
> > 
> > diff --git a/utils/v4l2-tracer/v4l2-tracer.cpp b/utils/v4l2-tracer/v4l2-tracer.cpp
> > index e3f002a9..7c3662be 100644
> > --- a/utils/v4l2-tracer/v4l2-tracer.cpp
> > +++ b/utils/v4l2-tracer/v4l2-tracer.cpp
> > @@ -5,6 +5,7 @@
> >  
> >  #include "retrace.h"
> >  #include <climits>
> > +#include <sys/stat.h>
> >  #include <sys/wait.h>
> >  #include <time.h>
> >  
> > @@ -295,24 +296,26 @@ int tracer(int argc, char *argv[], bool retrace)
> >  	fclose(trace_file);
> >  
> >  	/*
> > -	 * Preload the libv4l2tracer library. If the program is installed, load the library
> > -	 * from its installed location, otherwise load it locally. If it's loaded locally,
> > -	 * use ./configure --disable-dyn-libv4l.
> > +	 * Preload the libv4l2tracer library. The libv4l2tracer is looked up next to
> > +	 * the executable first in order to support uninstalled build.
> >  	 */
> >  	std::string libv4l2tracer_path;
> >  	std::string program = argv[0];
> > -	std::size_t idx = program.rfind("/v4l2-tracer");
> > -	if (idx != std::string::npos) {
> > -		libv4l2tracer_path = program.replace(program.begin() + idx + 1, program.end(), ".libs");
> > -		DIR *directory_pointer = opendir(libv4l2tracer_path.c_str());
> > -		if (directory_pointer == nullptr)
> > -			libv4l2tracer_path = program.replace(program.begin() + idx, program.end(), "./.libs");
> > -		else
> > -			closedir(directory_pointer);
> > -	} else {
> > -		libv4l2tracer_path = LIBTRACER_PATH;
> > -	}
> > -	libv4l2tracer_path += "/libv4l2tracer.so";
> > +	std::size_t idx = program.rfind("/");
> > +	struct stat sb;
> > +
> > +	if (idx == std::string::npos)
> > +		idx = 0;
> > +	else
> > +		idx++;
> > +
> > +	/* look for libv4l2tracer next to the executable */
> > +	libv4l2tracer_path = program.replace(program.begin() + idx, program.end(), "libv4l2tracer.so");
> > +
> > +	/* otherwise, use the installation path */
> > +	if (stat(libv4l2tracer_path.c_str(), &sb) == -1)
> > +		libv4l2tracer_path = std::string(LIBTRACER_PATH) + "/libv4l2tracer.so";
> > +
> >  	if (is_verbose())
> >  		fprintf(stderr, "Loading libv4l2tracer: %s\n", libv4l2tracer_path.c_str());
> >  	setenv("LD_PRELOAD", libv4l2tracer_path.c_str(), 0);
>