Re: [gentoo-dev] [PATCH] linux-info.eclass: get_version: remove useless readlink -f

2016-11-30 Thread Michael Orlitzky 
On 11/30/2016 11:45 AM, Mike Gilbert wrote:
> On Wed, Nov 30, 2016 at 11:28 AM, Michael Orlitzky  wrote:
>> On 11/26/2016 12:47 PM, Mike Gilbert wrote:
>>> The values get clobbered immediately afterward, so why bother?
>>> ...
>>>   qeinfo "Determining the location of the kernel source code"
>>> - [ -h "${KERNEL_DIR}" ] && KV_DIR="$(readlink -f ${KERNEL_DIR})"
>>>   [ -d "${KERNEL_DIR}" ] && KV_DIR="${KERNEL_DIR}"
>>>
>>
>> This changes the behavior if $KERNEL_DIR is a symbolic link to a
>> nonexistent directory, doesn't it?
> 
> Yes, I suppose it does. Do you anticipate that will cause a problem?
> 

*shrug*

There's a bug there, but who knows what the code was supposed to do. The
docs say that KV_DIR is

   ...a string containing the kernel source directory, will be null if
   KERNEL_DIR is invalid

To me, that makes the "readlink" line the bug, because KV_DIR will be
set even if KERNEL_DIR points nowhere. But then why is the readlink
there? Should KERNEL_DIR support symlinks? (Probably, yes.) If so, the
second test should be something like

  [ ! -d "${KV_DIR}" ] && KV_DIR=""

to undo the previous line when KERNEL_DIR didn't lead anywhere. A better
fix would avoid setting KV_DIR entirely in that case.

Re: [gentoo-dev] [PATCH] linux-info.eclass: get_version: remove useless readlink -f

2016-11-30 Thread Mike Gilbert 
On Wed, Nov 30, 2016 at 11:28 AM, Michael Orlitzky  wrote:
> On 11/26/2016 12:47 PM, Mike Gilbert wrote:
>> The values get clobbered immediately afterward, so why bother?
>> ...
>>   qeinfo "Determining the location of the kernel source code"
>> - [ -h "${KERNEL_DIR}" ] && KV_DIR="$(readlink -f ${KERNEL_DIR})"
>>   [ -d "${KERNEL_DIR}" ] && KV_DIR="${KERNEL_DIR}"
>>
>
> This changes the behavior if $KERNEL_DIR is a symbolic link to a
> nonexistent directory, doesn't it?

Yes, I suppose it does. Do you anticipate that will cause a problem?

Re: [gentoo-dev] [PATCH] linux-info.eclass: get_version: remove useless readlink -f

2016-11-30 Thread Michael Orlitzky 
On 11/26/2016 12:47 PM, Mike Gilbert wrote:
> The values get clobbered immediately afterward, so why bother?
> ...
>   qeinfo "Determining the location of the kernel source code"
> - [ -h "${KERNEL_DIR}" ] && KV_DIR="$(readlink -f ${KERNEL_DIR})"
>   [ -d "${KERNEL_DIR}" ] && KV_DIR="${KERNEL_DIR}"
>  

This changes the behavior if $KERNEL_DIR is a symbolic link to a
nonexistent directory, doesn't it?

[gentoo-dev] [PATCH] linux-info.eclass: get_version: remove useless readlink -f

2016-11-26 Thread Mike Gilbert 
The values get clobbered immediately afterward, so why bother?
---
 eclass/linux-info.eclass | 2 --
 1 file changed, 2 deletions(-)

diff --git a/eclass/linux-info.eclass b/eclass/linux-info.eclass
index 16740a3126fb..0dc7b0d0027b 100644
--- a/eclass/linux-info.eclass
+++ b/eclass/linux-info.eclass
@@ -445,7 +445,6 @@ get_version() {
# KV_DIR will contain the full path to the sources directory we should 
use
[ -z "${get_version_warning_done}" ] && \
qeinfo "Determining the location of the kernel source code"
-   [ -h "${KERNEL_DIR}" ] && KV_DIR="$(readlink -f ${KERNEL_DIR})"
[ -d "${KERNEL_DIR}" ] && KV_DIR="${KERNEL_DIR}"
 
if [ -z "${KV_DIR}" ]
@@ -539,7 +538,6 @@ get_version() {
done
fi
 
-   [ -h "${OUTPUT_DIR}" ] && KV_OUT_DIR="$(readlink -f ${OUTPUT_DIR})"
[ -d "${OUTPUT_DIR}" ] && KV_OUT_DIR="${OUTPUT_DIR}"
if [ -n "${KV_OUT_DIR}" ];
then
-- 
2.11.0.rc2