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/03/2011 07:25 PM, Cui, Dexuan wrote:
> Darren Hart wrote on 2011-08-03:
>> On 08/02/2011 11:46 PM, Cui, Dexuan wrote:
>>> 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
> Agree.
> 
>> 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%/}
> Hi Darren,
> This seems good to me.
> Looks ${BDIR%/} can only remove one trailing slash. Do we need to consider more-than-one-slashes, e.g., $BDIR is /home/poky/build///? :-)   (We could use sed:  BDIR=`echo $BDIR | sed -re 's|/+$||'` , but I'm not sure if it deserves the complexity)
> Darren, could you please help to make a patch? 
> I really have few experience about how to validate a user's input. :-)

At some point I think it's fair for us to say "so don't do that" when
someone says "if I pass this random string of garbage to the script it
gives up and uses the current directory".

As for a patch, I'm on Jury duty all this week and only get to a very
small percentage of my tasks. I would appreciate if you would try to put
one together using the above source snippet, with the suggested changes
from Paul of course. Then just send it to the list with Paul and myself
on CC. We'll review and provided additional Acked-by's to confirm we're
all happy with the end result.

I would suggest you include a patch to first revert the previous patch
that was applied to address this issue.

--
Darren

> 
>> 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.
> I agree.
> 
> Thanks,
> -- Dexuan
> 
> 

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