[PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Mike Looijmans]

The "command" shell command appears to be a bashism, the standard
busybox shell doesn't implement it.

This avoids the following error when logging in to a host that does
not have the 'command' command:

    -sh: command: not found

It also simplifies the code and reduces the number of forks.

Fixes: e77cdb761169e404556487ac650dc562000da406
Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 meta/recipes-core/base-files/base-files/profile | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
index ceaf15f..b5b533c 100644
--- a/meta/recipes-core/base-files/base-files/profile
+++ b/meta/recipes-core/base-files/base-files/profile
@@ -22,14 +22,12 @@ if [ -d /etc/profile.d ]; then
 	unset i
 fi
 
-if command -v resize >/dev/null && command -v tty >/dev/null; then
-	# Make sure we are on a serial console (i.e. the device used starts with
-	# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
-	# tries do use ssh
-	case $(tty) in
-		/dev/tty[A-z]*) resize >/dev/null;;
-	esac
-fi
+# Make sure we are on a serial console (i.e. the device used starts with
+# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
+# tries do use ssh
+case $(tty 2>/dev/null) in
+	/dev/tty[A-z]*) resize >/dev/null;;
+esac
 
 export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
 
-- 
1.9.1

[PATCH 2/2] base-files: profile: Make the "resize" command have the intended effect

[Mike Looijmans]

The "resize" command outputs shell script to be evaluated. The proper way
to use it is to do:
  eval `resize`
This sets the related environment variables COLUMNS and ROWS so that a vi
invokation uses the proper dimensions. Without the "eval" part, running
vi still gives a garbled screen when the dimensions aren't 80x25.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 meta/recipes-core/base-files/base-files/profile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
index b5b533c..36bd35f 100644
--- a/meta/recipes-core/base-files/base-files/profile
+++ b/meta/recipes-core/base-files/base-files/profile
@@ -26,7 +26,7 @@ fi
 # /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
 # tries do use ssh
 case $(tty 2>/dev/null) in
-	/dev/tty[A-z]*) resize >/dev/null;;
+	/dev/tty[A-z]*) eval `resize` ;;
 esac
 
 export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
-- 
1.9.1

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org
> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
> Mike Looijmans
> Sent: den 18 september 2017 10:39
> To: openembedded-core@lists.openembedded.org
> Cc: Mike Looijmans <mike.looijmans@topic.nl>
> Subject: [OE-core] [PATCH 1/2] base-files: profile: Do not assume that
> the 'command' command exists
> 
> The "command" shell command appears to be a bashism, the standard
> busybox shell doesn't implement it.
> 
> This avoids the following error when logging in to a host that does
> not have the 'command' command:
> 
>     -sh: command: not found
> 
> It also simplifies the code and reduces the number of forks.
> 
> Fixes: e77cdb761169e404556487ac650dc562000da406
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
>  meta/recipes-core/base-files/base-files/profile | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
> index ceaf15f..b5b533c 100644
> --- a/meta/recipes-core/base-files/base-files/profile
> +++ b/meta/recipes-core/base-files/base-files/profile
> @@ -22,14 +22,12 @@ if [ -d /etc/profile.d ]; then
>  	unset i
>  fi
> 
> -if command -v resize >/dev/null && command -v tty >/dev/null; then
> -	# Make sure we are on a serial console (i.e. the device used starts with
> -	# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
> -	# tries do use ssh
> -	case $(tty) in
> -		/dev/tty[A-z]*) resize >/dev/null;;
> -	esac
> -fi
> +# Make sure we are on a serial console (i.e. the device used starts with
> +# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
> +# tries do use ssh
> +case $(tty 2>/dev/null) in
> +	/dev/tty[A-z]*) resize >/dev/null;;
> +esac
> 
>  export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
> 
> --
> 1.9.1

This is basically the same change as I first sent a patch for in April, and 
last pinged this Friday... The only real difference is that this one misses 
passing error output from resize to /dev/null (which it should do to handle 
the case where tty exists, but resize does not).

//Peter

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Mike Looijmans]

On 18-09-17 10:49, Peter Kjellerstedt wrote:
>> 

Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail



-----Original Message-----
>> From: openembedded-core-bounces@lists.openembedded.org
>> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
>> Mike Looijmans
>> Sent: den 18 september 2017 10:39
>> To: openembedded-core@lists.openembedded.org
>> Cc: Mike Looijmans <mike.looijmans@topic.nl>
>> Subject: [OE-core] [PATCH 1/2] base-files: profile: Do not assume that
>> the 'command' command exists
>>
>> The "command" shell command appears to be a bashism, the standard
>> busybox shell doesn't implement it.
>>
>> This avoids the following error when logging in to a host that does
>> not have the 'command' command:
>>
>>      -sh: command: not found
>>
>> It also simplifies the code and reduces the number of forks.
>>
>> Fixes: e77cdb761169e404556487ac650dc562000da406
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>>   meta/recipes-core/base-files/base-files/profile | 14 ++++++--------
>>   1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
>> index ceaf15f..b5b533c 100644
>> --- a/meta/recipes-core/base-files/base-files/profile
>> +++ b/meta/recipes-core/base-files/base-files/profile
>> @@ -22,14 +22,12 @@ if [ -d /etc/profile.d ]; then
>>   	unset i
>>   fi
>>
>> -if command -v resize >/dev/null && command -v tty >/dev/null; then
>> -	# Make sure we are on a serial console (i.e. the device used starts with
>> -	# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
>> -	# tries do use ssh
>> -	case $(tty) in
>> -		/dev/tty[A-z]*) resize >/dev/null;;
>> -	esac
>> -fi
>> +# Make sure we are on a serial console (i.e. the device used starts with
>> +# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
>> +# tries do use ssh
>> +case $(tty 2>/dev/null) in
>> +	/dev/tty[A-z]*) resize >/dev/null;;
>> +esac
>>
>>   export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
>>
>> --
>> 1.9.1
> 
> This is basically the same change as I first sent a patch for in April, and
> last pinged this Friday... The only real difference is that this one misses
> passing error output from resize to /dev/null (which it should do to handle
> the case where tty exists, but resize does not).

Yeah, indeed.

Other problem is that "resize" outputs shell script on stdout to be executed, 
so the proper "total" invokation would be:

   /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;

The "eval" part is missing in your version...

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

On 18 September 2017 at 12:31, Mike Looijmans <mike.looijmans@topic.nl>
wrote:

> This is basically the same change as I first sent a patch for in April, and
>> last pinged this Friday... The only real difference is that this one
>> misses
>> passing error output from resize to /dev/null (which it should do to
>> handle
>> the case where tty exists, but resize does not).
>>
>
> Yeah, indeed.
>

Apologies for missing that patch!


> Other problem is that "resize" outputs shell script on stdout to be
> executed, so the proper "total" invokation would be:
>
>   /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
>
> The "eval" part is missing in your version...


Who is going to submit the One True patch with all the fixes in?  I promise
to merge it.

Ross

[-- Attachment #2: Type: text/html, Size: 1447 bytes --]

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Mike Looijmans]

On 18-09-17 15:08, Burton, Ross wrote:
> On 18 September 2017 at 12:31, Mike Looijmans <mike.looijmans@topic.nl 
> <mailto:mike.looijmans@topic.nl>> wrote:
> 
>         This is basically the same change as I first sent a patch for in
>         April, and
>         last pinged this Friday... The only real difference is that this one
>         misses
>         passing error output from resize to /dev/null (which it should do to
>         handle
>         the case where tty exists, but resize does not).
> 
> 
>     Yeah, indeed.
> 
> 
> Apologies for missing that patch!
> 
>     Other problem is that "resize" outputs shell script on stdout to be
>     executed, so the proper "total" invokation would be:
> 
>        /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
> 
>     The "eval" part is missing in your version...
> 
> 
> Who is going to submit the One True patch with all the fixes in?  I promise to 
> merge it.

I'll send the one ring, eh, patch, in a few minutes. I'll merge the two into a 
single as well.



Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Mike Looijmans]

On 18-09-17 15:24, Mike Looijmans wrote:
> On 18-09-17 15:08, Burton, Ross wrote:
>> On 18 September 2017 at 12:31, Mike Looijmans <mike.looijmans@topic.nl 
>> <mailto:mike.looijmans@topic.nl>> wrote:
>>
>>         This is basically the same change as I first sent a patch for in
>>         April, and
>>         last pinged this Friday... The only real difference is that this one
>>         misses
>>         passing error output from resize to /dev/null (which it should do to
>>         handle
>>         the case where tty exists, but resize does not).
>>
>>
>>     Yeah, indeed.
>>
>>
>> Apologies for missing that patch!
>>
>>     Other problem is that "resize" outputs shell script on stdout to be
>>     executed, so the proper "total" invokation would be:
>>
>>        /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
>>
>>     The "eval" part is missing in your version...
>>
>>
>> Who is going to submit the One True patch with all the fixes in?  I promise 
>> to merge it.
> 
> I'll send the one ring, eh, patch, in a few minutes. I'll merge the two into a 
> single as well.

On second thought, just use Peter's patch "as is".

I've been experimenting with the "eval" part and it doesn't behave well. Tends 
to confuse minicom, create garbage, and in particular when run from "profile", 
it seems to result in counterproductive COLUMNS=0 and LINES=0.

I'm actually wondering why the call to "resize" is being done at all. Just 
calling "resize" has no effect, since it outputs the results on stdout as 
shell script, and that is being discarded. Looking at the commit that 
introduced it:

cc6360f4c4d9 (base-files: set dynamic COLUMNS via resize command)

that already has no effect whatsoever. See the man page for resize:
https://linux.die.net/man/1/resize

I also would consider running some program's output as shell script a bit 
spooky, it looks like a security hole waiting to be exploited.



Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

[PATCH] base-files: profile: Get rid of "resize"

[Mike Looijmans]

The "resize" command actually outputs shell commands to be executed, for
example:

$ resize
COLUMNS=102;
LINES=27;
export COLUMNS LINES;

The output of "resize" is being discarded to /dev/null so the call has no
effect whatsoever, and does not change the environment (it cannot change the
environment of its parent). Remove the call and hence solve the messages
about shells missing "command" or "tty" or "resize".

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 meta/recipes-core/base-files/base-files/profile | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
index a062028..cfd0d69 100644
--- a/meta/recipes-core/base-files/base-files/profile
+++ b/meta/recipes-core/base-files/base-files/profile
@@ -20,15 +20,6 @@ if [ -d /etc/profile.d ]; then
 	unset i
 fi
 
-if command -v resize >/dev/null && command -v tty >/dev/null; then
-	# Make sure we are on a serial console (i.e. the device used starts with
-	# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
-	# tries do use ssh
-	case $(tty) in
-		/dev/tty[A-z]*) resize >/dev/null;;
-	esac
-fi
-
 export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
 
 umask 022
-- 
1.9.1

✗ patchtest: failure for "base-files: profile: Do not as..." and 1 more (rev2)

[Patchwork]

== Series Details ==

Series: "base-files: profile: Do not as..." and 1 more (rev2)
Revision: 2
URL   : https://patchwork.openembedded.org/series/8967/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at cbc396dd10)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [PATCH] base-files: profile: Get rid of "resize"

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org
> [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
> Mike Looijmans
> Sent: den 18 september 2017 16:07
> To: openembedded-core@lists.openembedded.org
> Cc: Mike Looijmans <mike.looijmans@topic.nl>; Peter Kjellerstedt
> <peter.kjellerstedt@axis.com>
> Subject: [OE-core] [PATCH] base-files: profile: Get rid of "resize"
> 
> The "resize" command actually outputs shell commands to be executed, for
> example:
> 
> $ resize
> COLUMNS=102;
> LINES=27;
> export COLUMNS LINES;
> 
> The output of "resize" is being discarded to /dev/null so the call has no
> effect whatsoever, and does not change the environment (it cannot change the
> environment of its parent). Remove the call and hence solve the messages
> about shells missing "command" or "tty" or "resize".
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> ---
>  meta/recipes-core/base-files/base-files/profile | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/meta/recipes-core/base-files/base-files/profile b/meta/recipes-core/base-files/base-files/profile
> index a062028..cfd0d69 100644
> --- a/meta/recipes-core/base-files/base-files/profile
> +++ b/meta/recipes-core/base-files/base-files/profile
> @@ -20,15 +20,6 @@ if [ -d /etc/profile.d ]; then
>  	unset i
>  fi
> 
> -if command -v resize >/dev/null && command -v tty >/dev/null; then
> -	# Make sure we are on a serial console (i.e. the device used starts with
> -	# /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher which
> -	# tries do use ssh
> -	case $(tty) in
> -		/dev/tty[A-z]*) resize >/dev/null;;
> -	esac
> -fi
> -
>  export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
> 
>  umask 022
> --
> 1.9.1

Hold on. Looking at busybox' source code for resize, it seems that 
it actually does something besides outputting the shell code to 
set the variables (which is actually enabled by a separate feature 
called ENABLE_FEATURE_RESIZE_PRINT). It also calls 
ioctl(STDERR_FILENO, TIOCSWINSZ, &w) where w contains the 
calculated sizes. 

However, I also just learnt that the busybox implementation of 
resize uses stderr to send control codes to move the cursor to 
find the tty sizes, which means that if one uses resize 2>/dev/null 
as my patch suggested, it does not work at all and just times out 
after three seconds.

What a mess...

//Peter

Re: [PATCH] base-files: profile: Get rid of "resize"

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 2577 bytes --]

On 18 September 2017 at 16:07, Peter Kjellerstedt <
peter.kjellerstedt@axis.com> wrote:

> > -----Original Message-----
> > From: openembedded-core-bounces@lists.openembedded.org
> > [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
> > Mike Looijmans
> > Sent: den 18 september 2017 16:07
> > To: openembedded-core@lists.openembedded.org
> > Cc: Mike Looijmans <mike.looijmans@topic.nl>; Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com>
> > Subject: [OE-core] [PATCH] base-files: profile: Get rid of "resize"
> >
> > The "resize" command actually outputs shell commands to be executed, for
> > example:
> >
> > $ resize
> > COLUMNS=102;
> > LINES=27;
> > export COLUMNS LINES;
> >
> > The output of "resize" is being discarded to /dev/null so the call has no
> > effect whatsoever, and does not change the environment (it cannot change
> the
> > environment of its parent). Remove the call and hence solve the messages
> > about shells missing "command" or "tty" or "resize".
> >
> > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> > ---
> >  meta/recipes-core/base-files/base-files/profile | 9 ---------
> >  1 file changed, 9 deletions(-)
> >
> > diff --git a/meta/recipes-core/base-files/base-files/profile
> b/meta/recipes-core/base-files/base-files/profile
> > index a062028..cfd0d69 100644
> > --- a/meta/recipes-core/base-files/base-files/profile
> > +++ b/meta/recipes-core/base-files/base-files/profile
> > @@ -20,15 +20,6 @@ if [ -d /etc/profile.d ]; then
> >       unset i
> >  fi
> >
> > -if command -v resize >/dev/null && command -v tty >/dev/null; then
> > -     # Make sure we are on a serial console (i.e. the device used
> starts with
> > -     # /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher
> which
> > -     # tries do use ssh
> > -     case $(tty) in
> > -             /dev/tty[A-z]*) resize >/dev/null;;
> > -     esac
> > -fi
> > -
> >  export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
> >
> >  umask 022
> > --
> > 1.9.1
>
> Hold on. Looking at busybox' source code for resize, it seems that
> it actually does something besides outputting the shell code to
> set the variables (which is actually enabled by a separate feature
> called ENABLE_FEATURE_RESIZE_PRINT). It also calls
> ioctl(STDERR_FILENO, TIOCSWINSZ, &w) where w contains the
> calculated sizes.
>

My knowledge of ANSI escapes is incredibly sketchy, but isn't that the code
to *get* the size of the screen?  Put the cursor at 999,999, then ask where
it is?

Ross

[-- Attachment #2: Type: text/html, Size: 3734 bytes --]

Re: [PATCH] base-files: profile: Get rid of "resize"

[Andre McCurdy]

On Mon, Sep 18, 2017 at 8:17 AM, Burton, Ross <ross.burton@intel.com> wrote:
> On 18 September 2017 at 16:07, Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
>>
>> > -----Original Message-----
>> > From: openembedded-core-bounces@lists.openembedded.org
>> > [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of
>> > Mike Looijmans
>> > Sent: den 18 september 2017 16:07
>> > To: openembedded-core@lists.openembedded.org
>> > Cc: Mike Looijmans <mike.looijmans@topic.nl>; Peter Kjellerstedt
>> > <peter.kjellerstedt@axis.com>
>> > Subject: [OE-core] [PATCH] base-files: profile: Get rid of "resize"
>> >
>> > The "resize" command actually outputs shell commands to be executed, for
>> > example:
>> >
>> > $ resize
>> > COLUMNS=102;
>> > LINES=27;
>> > export COLUMNS LINES;
>> >
>> > The output of "resize" is being discarded to /dev/null so the call has
>> > no
>> > effect whatsoever, and does not change the environment (it cannot change
>> > the
>> > environment of its parent). Remove the call and hence solve the messages
>> > about shells missing "command" or "tty" or "resize".
>> >
>> > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> > ---
>> >  meta/recipes-core/base-files/base-files/profile | 9 ---------
>> >  1 file changed, 9 deletions(-)
>> >
>> > diff --git a/meta/recipes-core/base-files/base-files/profile
>> > b/meta/recipes-core/base-files/base-files/profile
>> > index a062028..cfd0d69 100644
>> > --- a/meta/recipes-core/base-files/base-files/profile
>> > +++ b/meta/recipes-core/base-files/base-files/profile
>> > @@ -20,15 +20,6 @@ if [ -d /etc/profile.d ]; then
>> >       unset i
>> >  fi
>> >
>> > -if command -v resize >/dev/null && command -v tty >/dev/null; then
>> > -     # Make sure we are on a serial console (i.e. the device used
>> > starts with
>> > -     # /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher
>> > which
>> > -     # tries do use ssh
>> > -     case $(tty) in
>> > -             /dev/tty[A-z]*) resize >/dev/null;;
>> > -     esac
>> > -fi
>> > -
>> >  export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
>> >
>> >  umask 022
>> > --
>> > 1.9.1
>>
>> Hold on. Looking at busybox' source code for resize, it seems that
>> it actually does something besides outputting the shell code to
>> set the variables (which is actually enabled by a separate feature
>> called ENABLE_FEATURE_RESIZE_PRINT). It also calls
>> ioctl(STDERR_FILENO, TIOCSWINSZ, &w) where w contains the
>> calculated sizes.
>
> My knowledge of ANSI escapes is incredibly sketchy, but isn't that the code
> to *get* the size of the screen?  Put the cursor at 999,999, then ask where
> it is?

The behaviour may have got broken with the various /etc/profile
rewrites but it certainly did do something useful originally - it
greatly improves usability for shells run on serial consoles. Please
don't just remove it.

> Ross
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>

Re: [PATCH] base-files: profile: Get rid of "resize"

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 3270 bytes --]

On 18 September 2017 at 19:41, Andre McCurdy <armccurdy@gmail.com> wrote:

> On Mon, Sep 18, 2017 at 8:17 AM, Burton, Ross <ross.burton@intel.com>
> wrote:
> > On 18 September 2017 at 16:07, Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com> wrote:
> >>
> >> > -----Original Message-----
> >> > From: openembedded-core-bounces@lists.openembedded.org
> >> > [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf
> Of
> >> > Mike Looijmans
> >> > Sent: den 18 september 2017 16:07
> >> > To: openembedded-core@lists.openembedded.org
> >> > Cc: Mike Looijmans <mike.looijmans@topic.nl>; Peter Kjellerstedt
> >> > <peter.kjellerstedt@axis.com>
> >> > Subject: [OE-core] [PATCH] base-files: profile: Get rid of "resize"
> >> >
> >> > The "resize" command actually outputs shell commands to be executed,
> for
> >> > example:
> >> >
> >> > $ resize
> >> > COLUMNS=102;
> >> > LINES=27;
> >> > export COLUMNS LINES;
> >> >
> >> > The output of "resize" is being discarded to /dev/null so the call has
> >> > no
> >> > effect whatsoever, and does not change the environment (it cannot
> change
> >> > the
> >> > environment of its parent). Remove the call and hence solve the
> messages
> >> > about shells missing "command" or "tty" or "resize".
> >> >
> >> > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> >> > ---
> >> >  meta/recipes-core/base-files/base-files/profile | 9 ---------
> >> >  1 file changed, 9 deletions(-)
> >> >
> >> > diff --git a/meta/recipes-core/base-files/base-files/profile
> >> > b/meta/recipes-core/base-files/base-files/profile
> >> > index a062028..cfd0d69 100644
> >> > --- a/meta/recipes-core/base-files/base-files/profile
> >> > +++ b/meta/recipes-core/base-files/base-files/profile
> >> > @@ -20,15 +20,6 @@ if [ -d /etc/profile.d ]; then
> >> >       unset i
> >> >  fi
> >> >
> >> > -if command -v resize >/dev/null && command -v tty >/dev/null; then
> >> > -     # Make sure we are on a serial console (i.e. the device used
> >> > starts with
> >> > -     # /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher
> >> > which
> >> > -     # tries do use ssh
> >> > -     case $(tty) in
> >> > -             /dev/tty[A-z]*) resize >/dev/null;;
> >> > -     esac
> >> > -fi
> >> > -
> >> >  export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
> >> >
> >> >  umask 022
> >> > --
> >> > 1.9.1
> >>
> >> Hold on. Looking at busybox' source code for resize, it seems that
> >> it actually does something besides outputting the shell code to
> >> set the variables (which is actually enabled by a separate feature
> >> called ENABLE_FEATURE_RESIZE_PRINT). It also calls
> >> ioctl(STDERR_FILENO, TIOCSWINSZ, &w) where w contains the
> >> calculated sizes.
> >
> > My knowledge of ANSI escapes is incredibly sketchy, but isn't that the
> code
> > to *get* the size of the screen?  Put the cursor at 999,999, then ask
> where
> > it is?
>
> The behaviour may have got broken with the various /etc/profile
> rewrites but it certainly did do something useful originally - it
> greatly improves usability for shells run on serial consoles. Please
> don't just remove it.
>

The question is does it do something useful *now*?

Ross

[-- Attachment #2: Type: text/html, Size: 4969 bytes --]

Re: [PATCH] base-files: profile: Get rid of "resize"

[Andre McCurdy]

On Mon, Sep 18, 2017 at 11:43 AM, Burton, Ross <ross.burton@intel.com> wrote:
> On 18 September 2017 at 19:41, Andre McCurdy <armccurdy@gmail.com> wrote:
>>
>> On Mon, Sep 18, 2017 at 8:17 AM, Burton, Ross <ross.burton@intel.com>
>> wrote:
>> > On 18 September 2017 at 16:07, Peter Kjellerstedt
>> > <peter.kjellerstedt@axis.com> wrote:
>> >>
>> >> > -----Original Message-----
>> >> > From: openembedded-core-bounces@lists.openembedded.org
>> >> > [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf
>> >> > Of
>> >> > Mike Looijmans
>> >> > Sent: den 18 september 2017 16:07
>> >> > To: openembedded-core@lists.openembedded.org
>> >> > Cc: Mike Looijmans <mike.looijmans@topic.nl>; Peter Kjellerstedt
>> >> > <peter.kjellerstedt@axis.com>
>> >> > Subject: [OE-core] [PATCH] base-files: profile: Get rid of "resize"
>> >> >
>> >> > The "resize" command actually outputs shell commands to be executed,
>> >> > for
>> >> > example:
>> >> >
>> >> > $ resize
>> >> > COLUMNS=102;
>> >> > LINES=27;
>> >> > export COLUMNS LINES;
>> >> >
>> >> > The output of "resize" is being discarded to /dev/null so the call
>> >> > has
>> >> > no
>> >> > effect whatsoever, and does not change the environment (it cannot
>> >> > change
>> >> > the
>> >> > environment of its parent). Remove the call and hence solve the
>> >> > messages
>> >> > about shells missing "command" or "tty" or "resize".
>> >> >
>> >> > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> >> > ---
>> >> >  meta/recipes-core/base-files/base-files/profile | 9 ---------
>> >> >  1 file changed, 9 deletions(-)
>> >> >
>> >> > diff --git a/meta/recipes-core/base-files/base-files/profile
>> >> > b/meta/recipes-core/base-files/base-files/profile
>> >> > index a062028..cfd0d69 100644
>> >> > --- a/meta/recipes-core/base-files/base-files/profile
>> >> > +++ b/meta/recipes-core/base-files/base-files/profile
>> >> > @@ -20,15 +20,6 @@ if [ -d /etc/profile.d ]; then
>> >> >       unset i
>> >> >  fi
>> >> >
>> >> > -if command -v resize >/dev/null && command -v tty >/dev/null; then
>> >> > -     # Make sure we are on a serial console (i.e. the device used
>> >> > starts with
>> >> > -     # /dev/tty[A-z]), otherwise we confuse e.g. the eclipse
>> >> > launcher
>> >> > which
>> >> > -     # tries do use ssh
>> >> > -     case $(tty) in
>> >> > -             /dev/tty[A-z]*) resize >/dev/null;;
>> >> > -     esac
>> >> > -fi
>> >> > -
>> >> >  export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
>> >> >
>> >> >  umask 022
>> >> > --
>> >> > 1.9.1
>> >>
>> >> Hold on. Looking at busybox' source code for resize, it seems that
>> >> it actually does something besides outputting the shell code to
>> >> set the variables (which is actually enabled by a separate feature
>> >> called ENABLE_FEATURE_RESIZE_PRINT). It also calls
>> >> ioctl(STDERR_FILENO, TIOCSWINSZ, &w) where w contains the
>> >> calculated sizes.
>> >
>> > My knowledge of ANSI escapes is incredibly sketchy, but isn't that the
>> > code
>> > to *get* the size of the screen?  Put the cursor at 999,999, then ask
>> > where
>> > it is?
>>
>> The behaviour may have got broken with the various /etc/profile
>> rewrites but it certainly did do something useful originally - it
>> greatly improves usability for shells run on serial consoles. Please
>> don't just remove it.
>
>
> The question is does it do something useful *now*?

I don't have a board with a serial console to test at the moment.
Busybox hasn't changed in recent memory though, so as long as the
resize command does actually get run I presume it's still going to
work.

> Ross

Re: [PATCH] base-files: profile: Get rid of "resize"

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

On 18 September 2017 at 20:01, Andre McCurdy <armccurdy@gmail.com> wrote:

> > The question is does it do something useful *now*?
>
> I don't have a board with a serial console to test at the moment.
> Busybox hasn't changed in recent memory though, so as long as the
> resize command does actually get run I presume it's still going to
> work.
>

Re-read the code.  There's a call to ioctl with TIOCSWINSZ which I'm
*guessing* is set window size.  Google is not useful for this!  It does all
this on stderr.

So yes it *should* work, but not if you redirect stderr to /dev/null.

Yes, this is a mess. :)

Ross

[-- Attachment #2: Type: text/html, Size: 1641 bytes --]

Re: [PATCH] base-files: profile: Get rid of "resize"

[Andre McCurdy]

On Mon, Sep 18, 2017 at 12:19 PM, Burton, Ross <ross.burton@intel.com> wrote:
> On 18 September 2017 at 20:01, Andre McCurdy <armccurdy@gmail.com> wrote:
>>
>> > The question is does it do something useful *now*?
>>
>> I don't have a board with a serial console to test at the moment.
>> Busybox hasn't changed in recent memory though, so as long as the
>> resize command does actually get run I presume it's still going to
>> work.
>
> Re-read the code.

I've never read the code before, so don't make any claims to know
*how* it works or worked :-)

> There's a call to ioctl with TIOCSWINSZ which I'm
> *guessing* is set window size.  Google is not useful for this!  It does all
> this on stderr.
>
> So yes it *should* work, but not if you redirect stderr to /dev/null.

We don't redirect stderr to /dev/null, only stdout.

> Yes, this is a mess. :)
>
> Ross

Re: [PATCH] base-files: profile: Get rid of "resize"

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

On 18 September 2017 at 20:30, Andre McCurdy <armccurdy@gmail.com> wrote:

> On Mon, Sep 18, 2017 at 12:19 PM, Burton, Ross <ross.burton@intel.com>
> wrote:
> > On 18 September 2017 at 20:01, Andre McCurdy <armccurdy@gmail.com>
> wrote:
> >>
> >> > The question is does it do something useful *now*?
> >>
> >> I don't have a board with a serial console to test at the moment.
> >> Busybox hasn't changed in recent memory though, so as long as the
> >> resize command does actually get run I presume it's still going to
> >> work.
> >
> > Re-read the code.
>
> I've never read the code before, so don't make any claims to know
> *how* it works or worked :-)
>
> > There's a call to ioctl with TIOCSWINSZ which I'm
> > *guessing* is set window size.  Google is not useful for this!  It does
> all
> > this on stderr.
> >
> > So yes it *should* work, but not if you redirect stderr to /dev/null.
>
> We don't redirect stderr to /dev/null, only stdout.


One of the many patches here does.

I'm holding off applying any of them right now. :)

Ross

[-- Attachment #2: Type: text/html, Size: 1707 bytes --]

Re: [PATCH] base-files: profile: Get rid of "resize"

[Andre McCurdy]

On Mon, Sep 18, 2017 at 1:18 PM, Burton, Ross <ross.burton@intel.com> wrote:
>> > There's a call to ioctl with TIOCSWINSZ which I'm
>> > *guessing* is set window size.  Google is not useful for this!  It does
>> > all this on stderr.
>> >
>> > So yes it *should* work, but not if you redirect stderr to /dev/null.
>>
>> We don't redirect stderr to /dev/null, only stdout.
>
> One of the many patches here does.
>
> I'm holding off applying any of them right now. :)

The "get rid of resize" patch seems to have made it into mut.

Re: [PATCH] base-files: profile: Get rid of "resize"

[Trevor Woerner]

I use resize quasi-regularly. Ever end up in the situation where
you've run vim and now the command-line has no idea how large the
console is, so everything wraps or scrolls at the wrong times/places?
resize fixes that:

# . resize

Please don't remove it.

Re: [PATCH] base-files: profile: Get rid of "resize"

[Mike Looijmans]

On 18-09-17 20:43, Burton, Ross wrote:
> On 18 September 2017 at 19:41, Andre McCurdy <armccurdy@gmail.com 
> <mailto:armccurdy@gmail.com>> wrote:
> 
>     On Mon, Sep 18, 2017 at 8:17 AM, Burton, Ross <ross.burton@intel.com
>     <mailto:ross.burton@intel.com>> wrote:
>      > On 18 September 2017 at 16:07, Peter Kjellerstedt
>      > <peter.kjellerstedt@axis.com <mailto:peter.kjellerstedt@axis.com>> wrote:
>      >>
>      >> > 

Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail



-----Original Message-----
>      >> > From: openembedded-core-bounces@lists.openembedded.org
>     <mailto:openembedded-core-bounces@lists.openembedded.org>
>      >> > [mailto:openembedded-core-bounces@lists.openembedded.org
>     <mailto:openembedded-core-bounces@lists.openembedded.org>] On Behalf Of
>      >> > Mike Looijmans
>      >> > Sent: den 18 september 2017 16:07
>      >> > To: openembedded-core@lists.openembedded.org
>     <mailto:openembedded-core@lists.openembedded.org>
>      >> > Cc: Mike Looijmans <mike.looijmans@topic.nl
>     <mailto:mike.looijmans@topic.nl>>; Peter Kjellerstedt
>      >> > <peter.kjellerstedt@axis.com <mailto:peter.kjellerstedt@axis.com>>
>      >> > Subject: [OE-core] [PATCH] base-files: profile: Get rid of "resize"
>      >> >
>      >> > The "resize" command actually outputs shell commands to be executed, for
>      >> > example:
>      >> >
>      >> > $ resize
>      >> > COLUMNS=102;
>      >> > LINES=27;
>      >> > export COLUMNS LINES;
>      >> >
>      >> > The output of "resize" is being discarded to /dev/null so the call has
>      >> > no
>      >> > effect whatsoever, and does not change the environment (it cannot change
>      >> > the
>      >> > environment of its parent). Remove the call and hence solve the messages
>      >> > about shells missing "command" or "tty" or "resize".
>      >> >
>      >> > Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl
>     <mailto:mike.looijmans@topic.nl>>
>      >> > ---
>      >> >  meta/recipes-core/base-files/base-files/profile | 9 ---------
>      >> >  1 file changed, 9 deletions(-)
>      >> >
>      >> > diff --git a/meta/recipes-core/base-files/base-files/profile
>      >> > b/meta/recipes-core/base-files/base-files/profile
>      >> > index a062028..cfd0d69 100644
>      >> > --- a/meta/recipes-core/base-files/base-files/profile
>      >> > +++ b/meta/recipes-core/base-files/base-files/profile
>      >> > @@ -20,15 +20,6 @@ if [ -d /etc/profile.d ]; then
>      >> >       unset i
>      >> >  fi
>      >> >
>      >> > -if command -v resize >/dev/null && command -v tty >/dev/null; then
>      >> > -     # Make sure we are on a serial console (i.e. the device used
>      >> > starts with
>      >> > -     # /dev/tty[A-z]), otherwise we confuse e.g. the eclipse launcher
>      >> > which
>      >> > -     # tries do use ssh
>      >> > -     case $(tty) in
>      >> > -             /dev/tty[A-z]*) resize >/dev/null;;
>      >> > -     esac
>      >> > -fi
>      >> > -
>      >> >  export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
>      >> >
>      >> >  umask 022
>      >> > --
>      >> > 1.9.1
>      >>
>      >> Hold on. Looking at busybox' source code for resize, it seems that
>      >> it actually does something besides outputting the shell code to
>      >> set the variables (which is actually enabled by a separate feature
>      >> called ENABLE_FEATURE_RESIZE_PRINT). It also calls
>      >> ioctl(STDERR_FILENO, TIOCSWINSZ, &w) where w contains the
>      >> calculated sizes.
>      >
>      > My knowledge of ANSI escapes is incredibly sketchy, but isn't that the code
>      > to *get* the size of the screen?  Put the cursor at 999,999, then ask where
>      > it is?
> 
>     The behaviour may have got broken with the various /etc/profile
>     rewrites but it certainly did do something useful originally - it
>     greatly improves usability for shells run on serial consoles. Please
>     don't just remove it.
> 
> 
> The question is does it do something useful *now*?

My experience is that it does not, and never has. I run serial consoles all 
the time, and if they're not sized to 80x25 weird stuff happens.

Start vi on a serial terminal and it only uses the top 80x25. If a command 
exceed the 80 char mark, the cursor goes all weird.
Running "eval `resize`" fixes that (until you resize the window). Running just 
"resize" does not have any effect on that at all.

Cannot we determine the presence of "tty" and "resize" commands at compile 
time? something like:

if has_command("tty") and has_command("resize"):
     install -m 755 ${S}/resize.sh ${D}${sysconfdir}/profile.d/

Re: [PATCH] base-files: profile: Get rid of "resize"

[Mike Looijmans]

On 18-09-17 21:19, Burton, Ross wrote:
> On 18 September 2017 at 20:01, Andre McCurdy <armccurdy@gmail.com 
> <mailto:armccurdy@gmail.com>> wrote:
> 
>      > The question is does it do something useful *now*?
> 
>     I don't have a board with a serial console to test at the moment.
>     Busybox hasn't changed in recent memory though, so as long as the
>     resize command does actually get run I presume it's still going to
>     work.
> 
> 
> Re-read the code.  There's a call to ioctl with TIOCSWINSZ which I'm 
> *guessing* is set window size.  Google is not useful for this!  It does all 
> this on stderr.
> 

Google on TIOCSWINSZ yields:
https://linux.die.net/man/4/tty_ioctl

TIOCSWINSZ
const struct winsize *argp
     Set window size.

Of course, there is no explanation on what a "set window size" is supposed to 
accomplish on the terminal device.


> So yes it *should* work, but not if you redirect stderr to /dev/null.

Yep.


Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Re: [PATCH] base-files: profile: Get rid of "resize"

[Mike Looijmans]

On 19-09-17 01:07, Trevor Woerner wrote:
> I use resize quasi-regularly. Ever end up in the situation where
> you've run vim and now the command-line has no idea how large the
> console is, so everything wraps or scrolls at the wrong times/places?
> resize fixes that:
> 
> # . resize
> 
> Please don't remove it.
> 

We're not removing the "resize" command itself. It's about removing something 
that started as a simple one-liner in /etc/profile to call "resize" (that did 
not fix the problem) and grew out to be a monster over time.


Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Re: [PATCH] base-files: profile: Get rid of "resize"

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On 19 September 2017 at 06:26, Mike Looijmans <mike.looijmans@topic.nl>
wrote:

> My experience is that it does not, and never has. I run serial consoles
> all the time, and if they're not sized to 80x25 weird stuff happens.
>
> Start vi on a serial terminal and it only uses the top 80x25. If a command
> exceed the 80 char mark, the cursor goes all weird.
> Running "eval `resize`" fixes that (until you resize the window). Running
> just "resize" does not have any effect on that at all.
>
> Cannot we determine the presence of "tty" and "resize" commands at compile
> time? something like:
>
> if has_command("tty") and has_command("resize"):
>     install -m 755 ${S}/resize.sh ${D}${sysconfdir}/profile.d/


As part of busybox, yes.  Not anywhere else though.

Ross

[-- Attachment #2: Type: text/html, Size: 1194 bytes --]

Re: [PATCH] base-files: profile: Get rid of "resize"

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 209 bytes --]

On 18 September 2017 at 22:11, Andre McCurdy <armccurdy@gmail.com> wrote:

> The "get rid of resize" patch seems to have made it into mut.
>

It was reverted at 8pm last night but I didn't push.

Ross

[-- Attachment #2: Type: text/html, Size: 601 bytes --]

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[ChenQi]

On 09/18/2017 09:56 PM, Mike Looijmans wrote:
> On 18-09-17 15:24, Mike Looijmans wrote:
>> On 18-09-17 15:08, Burton, Ross wrote:
>>> On 18 September 2017 at 12:31, Mike Looijmans 
>>> <mike.looijmans@topic.nl <mailto:mike.looijmans@topic.nl>> wrote:
>>>
>>>         This is basically the same change as I first sent a patch 
>>> for in
>>>         April, and
>>>         last pinged this Friday... The only real difference is that 
>>> this one
>>>         misses
>>>         passing error output from resize to /dev/null (which it 
>>> should do to
>>>         handle
>>>         the case where tty exists, but resize does not).
>>>
>>>
>>>     Yeah, indeed.
>>>
>>>
>>> Apologies for missing that patch!
>>>
>>>     Other problem is that "resize" outputs shell script on stdout to be
>>>     executed, so the proper "total" invokation would be:
>>>
>>>        /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
>>>
>>>     The "eval" part is missing in your version...
>>>
>>>
>>> Who is going to submit the One True patch with all the fixes in?  I 
>>> promise to merge it.
>>
>> I'll send the one ring, eh, patch, in a few minutes. I'll merge the 
>> two into a single as well.
>
> On second thought, just use Peter's patch "as is".
>
> I've been experimenting with the "eval" part and it doesn't behave 
> well. Tends to confuse minicom, create garbage, and in particular when 
> run from "profile", it seems to result in counterproductive COLUMNS=0 
> and LINES=0.
>
> I'm actually wondering why the call to "resize" is being done at all. 
> Just calling "resize" has no effect, since it outputs the results on 
> stdout as shell script, and that is being discarded. Looking at the 
> commit that introduced it:
>

Looking at the codes in busybox (console-tools/resize.c), I can see that 
tcsetattr is actually called.
The output is due to:

     if (ENABLE_FEATURE_RESIZE_PRINT)
         printf("COLUMNS=%d;LINES=%d;export COLUMNS LINES;\n",
             w.ws_col, w.ws_row);

I can confirm that calling 'resize' is needed. Otherwise, you might get 
input wrapping on the same line when typing long commands.

I just tried busybox init based system and I also met '-sh: command: not 
found' error message.
I think Peter's patch ([OE-core] [PATCHv2 1/1] base-files: profile: 
Avoid using "command" to determine if programs exist) is correct.

Best Regards,
Chen Qi

> cc6360f4c4d9 (base-files: set dynamic COLUMNS via resize command)
>
> that already has no effect whatsoever. See the man page for resize:
> https://linux.die.net/man/1/resize
>
> I also would consider running some program's output as shell script a 
> bit spooky, it looks like a security hole waiting to be exploited.
>
>
>
> Kind regards,
>
> Mike Looijmans
> System Expert
>
> TOPIC Products
> Materiaalweg 4, NL-5681 RJ Best
> Postbus 440, NL-5680 AK Best
> Telefoon: +31 (0) 499 33 69 79
> E-mail: mike.looijmans@topicproducts.com
> Website: www.topicproducts.com
>
> Please consider the environment before printing this e-mail
>
>
>

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[ChenQi]

On 10/13/2017 05:53 PM, ChenQi wrote:
> On 09/18/2017 09:56 PM, Mike Looijmans wrote:
>> On 18-09-17 15:24, Mike Looijmans wrote:
>>> On 18-09-17 15:08, Burton, Ross wrote:
>>>> On 18 September 2017 at 12:31, Mike Looijmans 
>>>> <mike.looijmans@topic.nl <mailto:mike.looijmans@topic.nl>> wrote:
>>>>
>>>>         This is basically the same change as I first sent a patch 
>>>> for in
>>>>         April, and
>>>>         last pinged this Friday... The only real difference is that 
>>>> this one
>>>>         misses
>>>>         passing error output from resize to /dev/null (which it 
>>>> should do to
>>>>         handle
>>>>         the case where tty exists, but resize does not).
>>>>
>>>>
>>>>     Yeah, indeed.
>>>>
>>>>
>>>> Apologies for missing that patch!
>>>>
>>>>     Other problem is that "resize" outputs shell script on stdout 
>>>> to be
>>>>     executed, so the proper "total" invokation would be:
>>>>
>>>>        /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
>>>>
>>>>     The "eval" part is missing in your version...
>>>>
>>>>
>>>> Who is going to submit the One True patch with all the fixes in?  I 
>>>> promise to merge it.
>>>
>>> I'll send the one ring, eh, patch, in a few minutes. I'll merge the 
>>> two into a single as well.
>>
>> On second thought, just use Peter's patch "as is".
>>
>> I've been experimenting with the "eval" part and it doesn't behave 
>> well. Tends to confuse minicom, create garbage, and in particular 
>> when run from "profile", it seems to result in counterproductive 
>> COLUMNS=0 and LINES=0.
>>
>> I'm actually wondering why the call to "resize" is being done at all. 
>> Just calling "resize" has no effect, since it outputs the results on 
>> stdout as shell script, and that is being discarded. Looking at the 
>> commit that introduced it:
>>
>
> Looking at the codes in busybox (console-tools/resize.c), I can see 
> that tcsetattr is actually called.
> The output is due to:
>
>     if (ENABLE_FEATURE_RESIZE_PRINT)
>         printf("COLUMNS=%d;LINES=%d;export COLUMNS LINES;\n",
>             w.ws_col, w.ws_row);
>
> I can confirm that calling 'resize' is needed. Otherwise, you might 
> get input wrapping on the same line when typing long commands.
>
> I just tried busybox init based system and I also met '-sh: command: 
> not found' error message.

Sorry for the noise. Just noticed that oe-core has enabled 'command' for 
busybox by default. It's my own busybox defconfig that hasn't enabled it.

Best Regards,
Chen Qi

> I think Peter's patch ([OE-core] [PATCHv2 1/1] base-files: profile: 
> Avoid using "command" to determine if programs exist) is correct.
>
> Best Regards,
> Chen Qi
>
>> cc6360f4c4d9 (base-files: set dynamic COLUMNS via resize command)
>>
>> that already has no effect whatsoever. See the man page for resize:
>> https://linux.die.net/man/1/resize
>>
>> I also would consider running some program's output as shell script a 
>> bit spooky, it looks like a security hole waiting to be exploited.
>>
>>
>>
>> Kind regards,
>>
>> Mike Looijmans
>> System Expert
>>
>> TOPIC Products
>> Materiaalweg 4, NL-5681 RJ Best
>> Postbus 440, NL-5680 AK Best
>> Telefoon: +31 (0) 499 33 69 79
>> E-mail: mike.looijmans@topicproducts.com
>> Website: www.topicproducts.com
>>
>> Please consider the environment before printing this e-mail
>>
>>
>>
>

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 4397 bytes --]

On Fri, Oct 13, 2017 at 06:07:16PM +0800, ChenQi wrote:
> On 10/13/2017 05:53 PM, ChenQi wrote:
> > On 09/18/2017 09:56 PM, Mike Looijmans wrote:
> >> On 18-09-17 15:24, Mike Looijmans wrote:
> >>> On 18-09-17 15:08, Burton, Ross wrote:
> >>>> On 18 September 2017 at 12:31, Mike Looijmans 
> >>>> <mike.looijmans@topic.nl <mailto:mike.looijmans@topic.nl>> wrote:
> >>>>
> >>>>         This is basically the same change as I first sent a patch 
> >>>> for in
> >>>>         April, and
> >>>>         last pinged this Friday... The only real difference is that 
> >>>> this one
> >>>>         misses
> >>>>         passing error output from resize to /dev/null (which it 
> >>>> should do to
> >>>>         handle
> >>>>         the case where tty exists, but resize does not).
> >>>>
> >>>>
> >>>>     Yeah, indeed.
> >>>>
> >>>>
> >>>> Apologies for missing that patch!
> >>>>
> >>>>     Other problem is that "resize" outputs shell script on stdout 
> >>>> to be
> >>>>     executed, so the proper "total" invokation would be:
> >>>>
> >>>>        /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
> >>>>
> >>>>     The "eval" part is missing in your version...
> >>>>
> >>>>
> >>>> Who is going to submit the One True patch with all the fixes in?  I 
> >>>> promise to merge it.
> >>>
> >>> I'll send the one ring, eh, patch, in a few minutes. I'll merge the 
> >>> two into a single as well.
> >>
> >> On second thought, just use Peter's patch "as is".
> >>
> >> I've been experimenting with the "eval" part and it doesn't behave 
> >> well. Tends to confuse minicom, create garbage, and in particular 
> >> when run from "profile", it seems to result in counterproductive 
> >> COLUMNS=0 and LINES=0.
> >>
> >> I'm actually wondering why the call to "resize" is being done at all. 
> >> Just calling "resize" has no effect, since it outputs the results on 
> >> stdout as shell script, and that is being discarded. Looking at the 
> >> commit that introduced it:
> >>
> >
> > Looking at the codes in busybox (console-tools/resize.c), I can see 
> > that tcsetattr is actually called.
> > The output is due to:
> >
> >     if (ENABLE_FEATURE_RESIZE_PRINT)
> >         printf("COLUMNS=%d;LINES=%d;export COLUMNS LINES;\n",
> >             w.ws_col, w.ws_row);
> >
> > I can confirm that calling 'resize' is needed. Otherwise, you might 
> > get input wrapping on the same line when typing long commands.
> >
> > I just tried busybox init based system and I also met '-sh: command: 
> > not found' error message.
> 
> Sorry for the noise. Just noticed that oe-core has enabled 'command' for 
> busybox by default. It's my own busybox defconfig that hasn't enabled it.

I was just hit by the same. resize and tty work fine, but command applet is
missing in my images.

Is enabling command applet in busybox now hard requirement for all the
builds or is anyone still planing to send follow-up on this?

At least to show some more meaningful error when it's not available?

"you probably need to fix your busybox defconfig, because some old thread on oe-core ML says so"

Cheers,

> Best Regards,
> Chen Qi
> 
> > I think Peter's patch ([OE-core] [PATCHv2 1/1] base-files: profile: 
> > Avoid using "command" to determine if programs exist) is correct.
> >
> > Best Regards,
> > Chen Qi
> >
> >> cc6360f4c4d9 (base-files: set dynamic COLUMNS via resize command)
> >>
> >> that already has no effect whatsoever. See the man page for resize:
> >> https://linux.die.net/man/1/resize
> >>
> >> I also would consider running some program's output as shell script a 
> >> bit spooky, it looks like a security hole waiting to be exploited.
> >>
> >>
> >>
> >> Kind regards,
> >>
> >> Mike Looijmans
> >> System Expert
> >>
> >> TOPIC Products
> >> Materiaalweg 4, NL-5681 RJ Best
> >> Postbus 440, NL-5680 AK Best
> >> Telefoon: +31 (0) 499 33 69 79
> >> E-mail: mike.looijmans@topicproducts.com
> >> Website: www.topicproducts.com
> >>
> >> Please consider the environment before printing this e-mail
> >>
> >>
> >>
> >
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 201 bytes --]

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Peter Kjellerstedt]

> -----Original Message-----
> From: Martin Jansa [mailto:martin.jansa@gmail.com]
> Sent: den 18 maj 2018 12:29
> To: ChenQi <Qi.Chen@windriver.com>
> Cc: Mike Looijmans <mike.looijmans@topic.nl>; openembedded-
> core@lists.openembedded.org; Peter Kjellerstedt
> <peter.kjellerstedt@axis.com>; Burton, Ross <ross.burton@intel.com>
> Subject: Re: [OE-core] [PATCH 1/2] base-files: profile: Do not assume
> that the 'command' command exists
> 
> On Fri, Oct 13, 2017 at 06:07:16PM +0800, ChenQi wrote:
> > On 10/13/2017 05:53 PM, ChenQi wrote:
> > > On 09/18/2017 09:56 PM, Mike Looijmans wrote:
> > >> On 18-09-17 15:24, Mike Looijmans wrote:
> > >>> On 18-09-17 15:08, Burton, Ross wrote:
> > >>>> On 18 September 2017 at 12:31, Mike Looijmans
> > >>>> <mike.looijmans@topic.nl <mailto:mike.looijmans@topic.nl>>
> wrote:
> > >>>>
> > >>>>         This is basically the same change as I first sent a
> patch
> > >>>> for in
> > >>>>         April, and
> > >>>>         last pinged this Friday... The only real difference is
> that
> > >>>> this one
> > >>>>         misses
> > >>>>         passing error output from resize to /dev/null (which it
> > >>>> should do to
> > >>>>         handle
> > >>>>         the case where tty exists, but resize does not).
> > >>>>
> > >>>>
> > >>>>     Yeah, indeed.
> > >>>>
> > >>>>
> > >>>> Apologies for missing that patch!
> > >>>>
> > >>>>     Other problem is that "resize" outputs shell script on
> stdout
> > >>>> to be
> > >>>>     executed, so the proper "total" invokation would be:
> > >>>>
> > >>>>        /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
> > >>>>
> > >>>>     The "eval" part is missing in your version...
> > >>>>
> > >>>>
> > >>>> Who is going to submit the One True patch with all the fixes in?
> I
> > >>>> promise to merge it.
> > >>>
> > >>> I'll send the one ring, eh, patch, in a few minutes. I'll merge
> the
> > >>> two into a single as well.
> > >>
> > >> On second thought, just use Peter's patch "as is".
> > >>
> > >> I've been experimenting with the "eval" part and it doesn't behave
> > >> well. Tends to confuse minicom, create garbage, and in particular
> > >> when run from "profile", it seems to result in counterproductive
> > >> COLUMNS=0 and LINES=0.
> > >>
> > >> I'm actually wondering why the call to "resize" is being done at
> all.
> > >> Just calling "resize" has no effect, since it outputs the results
> on
> > >> stdout as shell script, and that is being discarded. Looking at
> the
> > >> commit that introduced it:
> > >>
> > >
> > > Looking at the codes in busybox (console-tools/resize.c), I can see
> > > that tcsetattr is actually called.
> > > The output is due to:
> > >
> > >     if (ENABLE_FEATURE_RESIZE_PRINT)
> > >         printf("COLUMNS=%d;LINES=%d;export COLUMNS LINES;\n",
> > >             w.ws_col, w.ws_row);
> > >
> > > I can confirm that calling 'resize' is needed. Otherwise, you might
> > > get input wrapping on the same line when typing long commands.
> > >
> > > I just tried busybox init based system and I also met '-sh:
> command:
> > > not found' error message.
> >
> > Sorry for the noise. Just noticed that oe-core has enabled 'command'
> for
> > busybox by default. It's my own busybox defconfig that hasn't enabled
> it.
> 
> I was just hit by the same. resize and tty work fine, but command
> applet is missing in my images.
> 
> Is enabling command applet in busybox now hard requirement for all the
> builds or is anyone still planing to send follow-up on this?
> 
> At least to show some more meaningful error when it's not available?
> 
> "you probably need to fix your busybox defconfig, because some old
> thread on oe-core ML says so"
> 
> Cheers,

Wow, here's an old discussion I had forgotten all about. I actually 
do have an updated version of the patch that solves the problem by 
running resize twice, once with stderr piped to /dev/null to 
determine if the resize command exists, and once without stderr 
piped to /dev/null to actually do the resize (since resize uses 
stderr to determine the size of the tty).

I'll send the updated patch in a minute.

> > Best Regards,
> > Chen Qi
> >
> > > I think Peter's patch ([OE-core] [PATCHv2 1/1] base-files: profile:
> > > Avoid using "command" to determine if programs exist) is correct.
> > >
> > > Best Regards,
> > > Chen Qi
> > >
> > >> cc6360f4c4d9 (base-files: set dynamic COLUMNS via resize command)
> > >>
> > >> that already has no effect whatsoever. See the man page for
> resize:
> > >> https://linux.die.net/man/1/resize
> > >>
> > >> I also would consider running some program's output as shell
> script a
> > >> bit spooky, it looks like a security hole waiting to be exploited.
> > >>
> > >>
> > >>
> > >> Kind regards,
> > >>
> > >> Mike Looijmans
> > >> System Expert
> > >>
> > >> TOPIC Products
> > >> Materiaalweg 4, NL-5681 RJ Best
> > >> Postbus 440, NL-5680 AK Best
> > >> Telefoon: +31 (0) 499 33 69 79
> > >> E-mail: mike.looijmans@topicproducts.com
> > >> Website: www.topicproducts.com
> > >>
> > >> Please consider the environment before printing this e-mail
> 
> --
> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

//Peter

Re: [PATCH 1/2] base-files: profile: Do not assume that the 'command' command exists

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 5715 bytes --]

Awesome, thanks!

On Fri, May 18, 2018 at 6:51 PM Peter Kjellerstedt <
peter.kjellerstedt@axis.com> wrote:

> > -----Original Message-----
> > From: Martin Jansa [mailto:martin.jansa@gmail.com]
> > Sent: den 18 maj 2018 12:29
> > To: ChenQi <Qi.Chen@windriver.com>
> > Cc: Mike Looijmans <mike.looijmans@topic.nl>; openembedded-
> > core@lists.openembedded.org; Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com>; Burton, Ross <ross.burton@intel.com>
> > Subject: Re: [OE-core] [PATCH 1/2] base-files: profile: Do not assume
> > that the 'command' command exists
> >
> > On Fri, Oct 13, 2017 at 06:07:16PM +0800, ChenQi wrote:
> > > On 10/13/2017 05:53 PM, ChenQi wrote:
> > > > On 09/18/2017 09:56 PM, Mike Looijmans wrote:
> > > >> On 18-09-17 15:24, Mike Looijmans wrote:
> > > >>> On 18-09-17 15:08, Burton, Ross wrote:
> > > >>>> On 18 September 2017 at 12:31, Mike Looijmans
> > > >>>> <mike.looijmans@topic.nl <mailto:mike.looijmans@topic.nl>>
> > wrote:
> > > >>>>
> > > >>>>         This is basically the same change as I first sent a
> > patch
> > > >>>> for in
> > > >>>>         April, and
> > > >>>>         last pinged this Friday... The only real difference is
> > that
> > > >>>> this one
> > > >>>>         misses
> > > >>>>         passing error output from resize to /dev/null (which it
> > > >>>> should do to
> > > >>>>         handle
> > > >>>>         the case where tty exists, but resize does not).
> > > >>>>
> > > >>>>
> > > >>>>     Yeah, indeed.
> > > >>>>
> > > >>>>
> > > >>>> Apologies for missing that patch!
> > > >>>>
> > > >>>>     Other problem is that "resize" outputs shell script on
> > stdout
> > > >>>> to be
> > > >>>>     executed, so the proper "total" invokation would be:
> > > >>>>
> > > >>>>        /dev/tty[A-z]*) eval `resize 2>/dev/null` ;;
> > > >>>>
> > > >>>>     The "eval" part is missing in your version...
> > > >>>>
> > > >>>>
> > > >>>> Who is going to submit the One True patch with all the fixes in?
> > I
> > > >>>> promise to merge it.
> > > >>>
> > > >>> I'll send the one ring, eh, patch, in a few minutes. I'll merge
> > the
> > > >>> two into a single as well.
> > > >>
> > > >> On second thought, just use Peter's patch "as is".
> > > >>
> > > >> I've been experimenting with the "eval" part and it doesn't behave
> > > >> well. Tends to confuse minicom, create garbage, and in particular
> > > >> when run from "profile", it seems to result in counterproductive
> > > >> COLUMNS=0 and LINES=0.
> > > >>
> > > >> I'm actually wondering why the call to "resize" is being done at
> > all.
> > > >> Just calling "resize" has no effect, since it outputs the results
> > on
> > > >> stdout as shell script, and that is being discarded. Looking at
> > the
> > > >> commit that introduced it:
> > > >>
> > > >
> > > > Looking at the codes in busybox (console-tools/resize.c), I can see
> > > > that tcsetattr is actually called.
> > > > The output is due to:
> > > >
> > > >     if (ENABLE_FEATURE_RESIZE_PRINT)
> > > >         printf("COLUMNS=%d;LINES=%d;export COLUMNS LINES;\n",
> > > >             w.ws_col, w.ws_row);
> > > >
> > > > I can confirm that calling 'resize' is needed. Otherwise, you might
> > > > get input wrapping on the same line when typing long commands.
> > > >
> > > > I just tried busybox init based system and I also met '-sh:
> > command:
> > > > not found' error message.
> > >
> > > Sorry for the noise. Just noticed that oe-core has enabled 'command'
> > for
> > > busybox by default. It's my own busybox defconfig that hasn't enabled
> > it.
> >
> > I was just hit by the same. resize and tty work fine, but command
> > applet is missing in my images.
> >
> > Is enabling command applet in busybox now hard requirement for all the
> > builds or is anyone still planing to send follow-up on this?
> >
> > At least to show some more meaningful error when it's not available?
> >
> > "you probably need to fix your busybox defconfig, because some old
> > thread on oe-core ML says so"
> >
> > Cheers,
>
> Wow, here's an old discussion I had forgotten all about. I actually
> do have an updated version of the patch that solves the problem by
> running resize twice, once with stderr piped to /dev/null to
> determine if the resize command exists, and once without stderr
> piped to /dev/null to actually do the resize (since resize uses
> stderr to determine the size of the tty).
>
> I'll send the updated patch in a minute.
>
> > > Best Regards,
> > > Chen Qi
> > >
> > > > I think Peter's patch ([OE-core] [PATCHv2 1/1] base-files: profile:
> > > > Avoid using "command" to determine if programs exist) is correct.
> > > >
> > > > Best Regards,
> > > > Chen Qi
> > > >
> > > >> cc6360f4c4d9 (base-files: set dynamic COLUMNS via resize command)
> > > >>
> > > >> that already has no effect whatsoever. See the man page for
> > resize:
> > > >> https://linux.die.net/man/1/resize
> > > >>
> > > >> I also would consider running some program's output as shell
> > script a
> > > >> bit spooky, it looks like a security hole waiting to be exploited.
> > > >>
> > > >>
> > > >>
> > > >> Kind regards,
> > > >>
> > > >> Mike Looijmans
> > > >> System Expert
> > > >>
> > > >> TOPIC Products
> > > >> Materiaalweg 4, NL-5681 RJ Best
> > > >> Postbus 440, NL-5680 AK Best
> > > >> Telefoon: +31 (0) 499 33 69 79
> > > >> E-mail: mike.looijmans@topicproducts.com
> > > >> Website: www.topicproducts.com
> > > >>
> > > >> Please consider the environment before printing this e-mail
> >
> > --
> > Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com
>
> //Peter
>
>

[-- Attachment #2: Type: text/html, Size: 8791 bytes --]