[Buildroot] [PATCH] support/dependencies/dependencies.sh: fix check for CWD in env vars

[Buildroot] [PATCH] support/dependencies/dependencies.sh: fix check for CWD in env vars

[Juan Garcia Blanco]

The previous pattern incorrectly matches against path lists that don't
contain CWD, but end with ':', e.g., exported like VAR=<...>:$VAR where
$VAR is empty.

Now the pattern explicitly checks for $PWD in the path list.

Signed-off-by: Juan Garcia Blanco <juanrgar@gmail.com>
---
 support/dependencies/dependencies.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index c604a9efcc..d1fe94c9b9 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -16,7 +16,7 @@ fi
 
 # Sanity check for CWD in LD_LIBRARY_PATH
 case ":${LD_LIBRARY_PATH:-unset}:" in
-(*::*|*:.:*)
+(*:$PWD:*|*:.:*)
 	echo
 	echo "You seem to have the current working directory in your"
 	echo "LD_LIBRARY_PATH environment variable. This doesn't work."
@@ -32,7 +32,7 @@ esac
 # An empty PATH is technically possible, but in practice we would not
 # even arrive here if that was the case.
 case ":${PATH:-unset}:" in
-(*::*|*:.:*)
+(*:$PWD:*|*:.:*)
 	echo
 	echo "You seem to have the current working directory in your"
 	echo "PATH environment variable. This doesn't work."
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] support/dependencies/dependencies.sh: fix check for CWD in env vars

[Yann E. MORIN]

Juan, All,

Thanks for following up on our earlier discussion on IRC.

On 2022-03-19 22:45 +0100, Juan Garcia Blanco spake thusly:
> The previous pattern incorrectly matches against path lists that don't
> contain CWD, but end with ':', e.g., exported like VAR=<...>:$VAR where
> $VAR is empty.
> 
> Now the pattern explicitly checks for $PWD in the path list.
> 
> Signed-off-by: Juan Garcia Blanco <juanrgar@gmail.com>
> ---
>  support/dependencies/dependencies.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
> index c604a9efcc..d1fe94c9b9 100755
> --- a/support/dependencies/dependencies.sh
> +++ b/support/dependencies/dependencies.sh
> @@ -16,7 +16,7 @@ fi
>  
>  # Sanity check for CWD in LD_LIBRARY_PATH
>  case ":${LD_LIBRARY_PATH:-unset}:" in
> -(*::*|*:.:*)
> +(*:$PWD:*|*:.:*)

Nope, that is not correct. This checks that indeed PWD *at time of
check* is not in LD_LIBRARY_PATH (the following applies equally to PATH
and LD_LIBRARY_PATH)

However, what we want to check for, is that PWD *at time of use* is not
in LD_LIBRARY_PATH.

Consider a package build system that would do something like (cross-gcc
is linked to libgmp.so):

    cd src
    cross-gcc -shared -o libgmp.so my-gmp.c
    cross-gcc -o foo foo.c

When gcc is called, PWD is no longer the same as it was when Buildroot
checked for it, so when running cross-gcc, the dynamic linker will try
to load libgmp.so from the current direc tory, and as it exists, it will
load the incorrect libgmp.so. And this is exactly what we want to avoid
and what we want to check for.

For PATH, an empty entry is explicitly documented [0] as the same as
'.', but for LD_LIBRARY_PATH, it is not explicitly documented what an
empty entry would do, so when I wrote the code above, I erred on the
safe side and also considered an empty entry to be equivalent to '.'
for LD_LIBRARY_PATH too.

So we really, really need to keep a check for an empty entry.

However, as we discussed on IRC, the checks really have an issue, in
that they do not test for a '.' or an empty entrey at the beginnning or
the end of PATH or LD_LIBRARY_PATH.

So both test patterns must be changed to:

    *::*|:*|*:|*:.:*|.:*|*:.

Could you send an updated patch, please?

[0] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

    A zero-length prefix is a legacy feature that indicates the current
    working directory. It appears as two adjacent <colon> characters
    ( "::" ), as an initial <colon> preceding the rest of the list, or
    as a trailing <colon> following the rest of the list.

Regards,
Yann E. MORIN.

>  	echo
>  	echo "You seem to have the current working directory in your"
>  	echo "LD_LIBRARY_PATH environment variable. This doesn't work."
> @@ -32,7 +32,7 @@ esac
>  # An empty PATH is technically possible, but in practice we would not
>  # even arrive here if that was the case.
>  case ":${PATH:-unset}:" in
> -(*::*|*:.:*)
> +(*:$PWD:*|*:.:*)
>  	echo
>  	echo "You seem to have the current working directory in your"
>  	echo "PATH environment variable. This doesn't work."
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot