[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gentoo-dev
Subject:    Re: [gentoo-dev] [PATCH] linux-info.eclass: get_version: remove useless readlink -f
From:       Michael Orlitzky <mjo () gentoo ! org>
Date:       2016-11-30 17:05:37
Message-ID: 1752260d-bf23-7d8e-08ca-1ddb427dc455 () gentoo ! org
[Download RAW message or body]

On 11/30/2016 11:45 AM, Mike Gilbert wrote:
> On Wed, Nov 30, 2016 at 11:28 AM, Michael Orlitzky <mjo@gentoo.org> 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.


[prev in list] [next in list] [prev in thread] [next in thread]