Re: [PATCH 1/1] oe-init-build-env, scripts/oe-buildenv-internal: add error detecting for $BDIR

[Darren Hart <dvhart@linux.intel.com>]

On 08/02/2011 11:46 PM, Cui, Dexuan wrote:
> Darren Hart wrote on 2011-08-03:
>> On 08/02/2011 04:43 AM, Richard Purdie wrote:
>>> On Tue, 2011-08-02 at 14:08 +0800, Dexuan Cui wrote:
>>>> [YOCTO #671]
>>> 
>> For a patch to address a relatively benign bug I thought the
>> standard procedure would be for it to await feedback for more than
>> 5 hours. I
> I agree. :-)
> 
>> was hoping to have an opportunity to review this fix as I was
>> working with the team in root causing the bug.
>> 
>> + if [ -z "$BDIR" ]; then + if expr "$1" : '.*/$' >/dev/null; then
>> echo >&2 "Error: please + remove any trailing / in the argument."
>> 
>> This portion of the patch is really not necessary. There is no 
>> meaningful difference between the path with or without the
>> trailing slash, a much simpler and less noisy solution would be to
>> simply strip the trailing slash from the user input before doing
>> the rest of the input validation.
> Hi Darren, thanks for the suggestion! I considered the idea too,
> however, if we use the idea, it looks not that simple to gracefully
> and concisely handle the case if a user (by accident or by prank)
> passes / as $1 here, i.e., "readlink -f" would fail. So I didn't do
> that.

Hi Dexuan,

I had not considered that case, good catch. I can't think of a valid use
case for BDIR="/". Not only are write permissions unlikely, but the
build would conflict with /tmp as well.

if [ "$BDIR" == "/" ]; then
	echo "ERROR: / is not supported as a build directory."
	exit 1
fi
BDIR=${BDIR%/}

I would be happy with something like the above (untested). It seems a
lot more clear and direct to me.

In any case, I don't see any reason to bail out and ask the user to
remove a trailing slash. We should just do this and move on. There is no
semantic difference from the user's perspective, and the blame is to be
placed on readlink, not the user.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel