Re: Re busybox tar hidden filename exploit

[Kang-Che Sung]

(Sorry if my mail client messes up with the quoting. I use the mobile web
version of Gmail.)

Steffen Nurpmeso  於 2024年7月4日 星期四寫道：
>  |Kang-Che Sung wrote in
>  |
>  |Just FYI, there is a portable alternative to the $''
(dollar-single-quote)
>  |of passing special characters in the shell. It's $(printf '...') with
>  |command substitution.
>
> You mean the %q format?  That is not standardized.
>
>%q ARGUMENT is printed in a format that can be reused as  shell
in-
>   put,  escaping  non-printable  characters with the proposed
POSIX
>   $'' syntax.
>
> Just like bash(1)s ${parameter@operator}:
>
> Q  The expansion is a string that is the value  of  parameter
>quoted in a format that can be reused as input.

I am not expecting any quoted and shell-escaped filename output may be
reused as input. Such quoting and escaping may be useful in filtering
well-known problematic characters (shell meta-characters, quotation marks,
etc.), but would never be complete in mitigating all possible attacks with
the Unicode characters.

That's why I mentioned two use cases, and made them distinct. You can't win
both.

> Well one could look for isatty(3) for example.
> Things are easier if you also know you are in a Unicode-aware
> environment, then you can simply add U+2400 aka do
>
>  if(!iswprint(wc) && wc != '\n' /*&& wc != '\r' && wc != '\b'*/ &&
>wc != '\t'){
> if ((wc & ~S(wchar_t,037)) == 0)
>wc = isuni ? 0x2400 | wc : '?';
> else if(wc == 0177)
>wc = isuni ? 0x2421 : '?';
> else
>wc = isuni ? 0x2426 : '?';
>
> but in other cases have to be aware of L-TO-R and R-TO-R marks,
> zero width and non-characters, ie most brutal (where isuni tells
> us that the character set aka wchar_t is real Unicode).
>
>}else if(isuni){ /* TODO ctext */
>   /* Need to filter out L-TO-R and R-TO-R marks TODO ctext */
>   if(wc == 0x200E || wc == 0x200F || (wc >= 0x202A && wc <=
0x202E))
>  continue;
>   /* And some zero-width messes */
>   if(wc == 0x00AD || (wc >= 0x200B && wc <= 0x200D))
>  continue;
>   /* Oh about the ISO C wide character interfaces, baby! */
>   if(wc == 0xFEFF)
>  continue;
>}

This was the second use case I mentioned. That is,
`--quoting-style=whatever`. We can make this the default when `stdout` is a
terminal, and I believe GNU utilities already did this.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re busybox tar hidden filename exploit

[Kang-Che Sung]

Steffen Nurpmeso  於 2024年7月3日 星期三寫道：
> Kang-Che Sung wrote in

>  |When it comes to unusual filenames, the GNU way of doing it is
implementing
>  |a `--null` option that accepts the list of filenames separated by ASCII
NUL
>  |characters.
>  |
>  |Various other utilities can print the filename list with NUL as the
>  |separator. For example `-print0` command in `find(1)`.
>
> This (at least, too lazy to look) is also part of the new POSIX
> standard released in June.  Ie, going that NUL thing seems "to
> come", it *could* be that there are other issues lying around for
> the next standard.
>
>   ...
>
> (Nonetheless quoting in the shell language is a must
>
>   80092  The application shall quote the following characters if
they are to represent themselves:
>   80093  |& ;<>   ()$`\ "
'   
>
> and POSIX 2024 adds the $'' dollar single quote mechanism (dash is
> about to implement it / has just recently done so), and for tools
> producing output for the (interaction with the) shell that thus
> seems useful to have; i do not know how portable "IFS= xy" is..)
>

Just FYI, there is a portable alternative to the $'' (dollar-single-quote)
of passing special characters in the shell. It's $(printf '...') with
command substitution.

It is useful if the special characters are known ahead of time, and it's
not a complete substitute of `ls --quoting-style=shell` nor `ls --zero`.

I'm not sure what the use case of the original reporter (Ian Norton) is,
but it's simply not part of the goal for `tar -tf foo.tar` to output or
escape special characters in filenames.

In other words, there's no bug here, just a UX inconvenience that special
characters are not displayed properly.

* If you want a secure protocol for outputting filenames or accepting
filenames in tar(1) and other utilities, then the `--null` option is the
way to go. Human readability of the filenames is second for this use case.
* If you want outputting filenames with human readability and all special
characters escaped, then GNU tar has the `--quoting-style` option that
busybox can consider implementing too, but keep in mind that this is meant
for _output_ only, not for secure _input_ of filenames. (Besides, I don't
know if it would escape problematic Unicode control characters. There was a
Unicode Bidi vulnerability nicknamed "Trojan Source" that you might be
interested in knowing.)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Re busybox tar hidden filename exploit

[Kang-Che Sung]

From: Ian Norton
>
> I had a brief look at if I could submit a patch, but I'm very very new to
the busybox codebase.  It appears that the same functions used to print the
filenames to stdout are also shared by a number of other busybox modules.
I _think_ that the cpio tool has the same flaw.
>
> Something that would escape any non-ascii would have been my first
instinct too though perhaps that would not work so well on non 8-bit
charsets.

From: Walter Harms
>
> What does gnutar do here  ?
>

Hey. Did you guys read this section in GNN find manual?

"Safe File Name Handling"
https://www.gnu.org/software/findutils/manual/html_node/find_html/Safe-File-Name-Handling.html

When it comes to unusual filenames, the GNU way of doing it is implementing
a `--null` option that accepts the list of filenames separated by ASCII NUL
characters.

Various other utilities can print the filename list with NUL as the
separator. For example `-print0` command in `find(1)`.

These are how GNU utilities handle unusual filenames.

It might not sound like a good idea to implement something that escape
filenames during output (such as GNU ls(1) `--quoting-style` option),
because a lot of utilities would need it, and I believe it's not the Unix
way of doing things. Filename quoting and escaping should ideally be it's
own utility (the closest I could find is `od -c`, although it's not in the
quoting style people are familiar with).
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] switch_root: remove /init check

[Kang-Che Sung]

Linus Heckemann  於 2024年4月23日 星期二寫道：
>
> I don't really see the existence of /init being so critical for
> this check given that we check below that it's a ramfs or tmpfs, which
> seems to me to be enough that people won't be destroying filesystems
> they cared a great deal about.
>

The ramfs/tmpfs check was good as it won't destroy any _permanent_ file the
user could have, but it wouldn't hurt either when there is another sanity
check.


>> Perhaps a better approach is to check the existence of what's specified
in
>> the "rdinit" parameter instead.
>
> That would introduce an additional dependency on /proc being mounted and
> require additional parsing. I don't think the check is that necessary,
> again because we have the /-is-ramfs-or-tmpfs check. But if you do think
> we need it I can rewrite the patch to check for rdinit= on cmdline as
well.
>

/proc should be mounted by most init systems anyway. But we can skip the
check when /proc doesn't exist, just in case.

The logic would be roughly like this:

If "/proc/cmdline" exists
   Read the "rdinit" parameter from "/proc/cmdline"; if it's unspecified,
default to "/init"
   If the file in "rdinit" doesn't exist, stop.
Else
   Skip the "rdinit" existence check and continue the switch_root process
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] switch_root: remove /init check

[Kang-Che Sung]

Linus Heckemann  於 2024年4月19日 星期五寫道：
> We were having some difficulty switching out of our custom initramfs
> into the final filesystem, with the error "message '/init' is not a
> regular file". We were confused as to why it was looking for `/init`
> -- we didn't have `/init`, neither in our initrd nor in the
> destination filesystem -- we were using the rdinit= command line
> parameter to provide an alternative path for the init in the initrd,
> and the target init was determined by userspace.
>
> Thankfully, the error message was fairly clear and easy to find in the
> source!
>
> We thus propose removing this check, since not all initrds have their
> init at /init -- setting rdinit= on the kernel command line allows
> using an alternative path. Thus, this check can prevent switching root
> in a scenario where it would be perfectly appropriate.
> ---
>  util-linux/switch_root.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/util-linux/switch_root.c b/util-linux/switch_root.c
> index 14139736e..ef6669f5c 100644
> --- a/util-linux/switch_root.c
> +++ b/util-linux/switch_root.c
> @@ -238,9 +238,6 @@ int switch_root_main(int argc UNUSED_PARAM, char
**argv)
> // Additional sanity checks: we're about to rm -rf /, so be
REALLY SURE
> // we mean it. I could make this a CONFIG option, but I would get
email
> // from all the people who WILL destroy their filesystems.
> -   if (stat("/init", ) != 0 || !S_ISREG(st.st_mode)) {
> -   bb_error_msg_and_die("'%s' is not a regular file",
"/init");
> -   }
> statfs("/", ); // this never fails
> if ((unsigned)stfs.f_type != RAMFS_MAGIC
>  && (unsigned)stfs.f_type != TMPFS_MAGIC
> --
> 2.42.0

Did you read the code comments on the lines above what you deleted? It's a
sanity check before deleting everything on the "/" filesystem.

Perhaps a better approach is to check the existence of what's specified in
the "rdinit" parameter instead.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2] bloat-o-meter: line wrapped at 75 columns instead of 80

[Kang-Che Sung]

Jones Syue 薛懷宗  於 2024年3月11日 星期一寫道：
> This patch replaces the 78 "-" prints with 75 "-". And replace the 80
> columns summary line with 77 columns. ("%s" is considered as two chars
> and should be filled with whitespace " ", so 77 = 75 + 2)
>
> Consider this scenario: a patch contains the output of "bloat-o-meter" to
> clarify about the size impact/diff, and when we validate this patch with
> "~/linux/scripts/checkpatch.pl" (from linux kernel source tree), which
> checks for style violations, it might complain about line wrapped like:[1]
> WARNING: \
> Possible unwrapped commit description (prefer a maximum 75 chars per line)
>
> The 1st complaint is seperation line with 78 '-' prints, and the 2nd
> complaint is summary line "(add/remove ... Total: n bytes)". Although
> these two warnings are not harmful at all, it is helpful and makes life
> easier if this kind of patch (with "bloat-o-meter" output) could be passed
> by 'checkpatch.pl' in the first place without manually inspection.
>
> [1] line wrapped at 75 columns:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> Signed-off-by: Jones Syue 
> ---
> v2:
>   - fix nit/typo with correct word 'scenario'
> v1: http://lists.busybox.net/pipermail/busybox/2024-March/090656.html
> ---
>  scripts/bloat-o-meter | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

I was curious. Is there a reason for BusyBox's bloat-o-meter script not to
keep in sync with the version that comes in the Linux kernel source?

I occasionally use the bloat-o-meter from the Linux kernel to compare even
BusyBox binaries. There shouldn't be any functional differences between the
two versions.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] seedrng: fix getrandom() detection for non-glibc libc

[Kang-Che Sung]

Just my two cents...

On Wednesday, April 19, 2023, Raphaël Mélotte 
wrote:
> +# Not all libc versions have getrandom, so check for it.
> +HAVE_GETRANDOM := $(shell printf '#include \n#include
\nint main(void){char
buf[256];\ngetrandom(buf,sizeof(buf),GRND_NONBLOCK);}' >bb_libtest.c; $(CC)
$(CFLAGS) $(CFLAGS_busybox) -D_GNU_SOURCE -o /dev/null bb_libtest.c
>/dev/null 2>&1 && echo "y"; rm bb_libtest.c)

How about putting the define line '#define _GNU_SOURCE' into bb_libtest.c ?
I don't like defining the feature test macros externally in '-D' options.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: sha256sum Illegal instruction on musl amd64

[Kang-Che Sung]

On Fri, Mar 31, 2023 at 12:34 AM  wrote:
>
> shaNI = ((ebx >> 28) & 2) - 1; /* bit 29 -> 1 or -1 */
>
> Seems a lot more complicated than intel's approach:
>
> shaNI = ((ebx >> 29) & 1);
>

The `shaNI` variable is not a boolean, but has three possible values:
0 (undetermined), 1 (SHA instructions supported), -1 (not supported)
That's why the slightly complicated logic.

By the way, the `shaNI` name is a misnomer, as Intel never uses "NI"
to refer to their SHA instruction set, unlike the AES instruction set.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] find: implement -nouser, -nogroup

[Kang-Che Sung]

On Sunday, January 29, 2023, David Leonard <
d+busy...@adaptive-enterprises.com> wrote:
>
> Resending patch for 'find -nouser', 'find -nogroup'. Refreshed bloatcheck
>
> Subject: [PATCH] find: implement -nouser, -nogroup
>
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/find.html
>
>   -nouser
> The primary shall evaluate as true if the file belongs to a user ID
> for which the getpwuid() function defined in the System Interfaces
> volume of POSIX.1-2017 (or equivalent) returns NULL.
>
>   -nogroup
> The primary shall evaluate as true if the file belongs to a group ID
> for which the getgrgid() function defined in the System Interfaces
> volume of POSIX.1-2017 (or equivalent) returns NULL.
>
> function old new   delta
> parse_params18111845 +34
> func_nouser-  24 +24
> func_nogroup   -  24 +24
> static.params275 292 +17
> .rodata   100767  100775  +8
> packed_usage   34553   34541 -12
>
--
> (add/remove: 2/0 grow/shrink: 3/1 up/down: 107/-12)Total: 95
bytes
>textdata bss dec hex filename
> 1064435   165871816 1082838  1085d6 busybox_old
> 1064530   165871816 1082933  108635 busybox_unstripped
> ---
>  findutils/find.c | 46 ++
>  1 file changed, 46 insertions(+)
>
> diff --git a/findutils/find.c b/findutils/find.c
> index 40f66ab2e..2a0a867e3 100644
> --- a/findutils/find.c
> +++ b/findutils/find.c
> @@ -197,6 +197,16 @@
>  //config:  default y
>  //config:  depends on FIND
>  //config:
> +//config:config FEATURE_FIND_NOUSER
> +//config:  bool "Enable -nouser matching"
> +//config:  default y
> +//config:  depends on FIND
> +//config:
> +//config:config FEATURE_FIND_NOGROUP
> +//config:  bool "Enable -nogroup matching"
> +//config:  default y
> +//config:  depends on FIND
> +//config:
>  //config:config FEATURE_FIND_NOT
>  //config:  bool "Enable the 'not' (!) operator"
>  //config:  default y
> @@ -373,6 +383,12 @@
>  //usage:   IF_FEATURE_FIND_GROUP(
>  //usage: "\n   -group NAME/ID  File is owned by given group"
>  //usage:   )
> +//usage:   IF_FEATURE_FIND_NOUSER(
> +//usage: "\n   -nouser File is owned by unknown uid"
> +//usage:   )
> +//usage:   IF_FEATURE_FIND_NOGROUP(
> +//usage: "\n   -nogroupFile is owned by unknown gid"
> +//usage:   )
>  //usage:   IF_FEATURE_FIND_SIZE(
>  //usage: "\n   -size N[bck]File size is N
(c:bytes,k:kbytes,b:512 bytes(def.))"
>  //usage: "\n   +/-N: file size is bigger/smaller
than N"
> @@ -466,6 +482,8 @@ IF_FEATURE_FIND_NEWER(  ACTS(newer, time_t
newer_mtime;))
>  IF_FEATURE_FIND_INUM(   ACTS(inum,  ino_t inode_num;))
>  IF_FEATURE_FIND_SAMEFILE(ACTS(samefile, ino_t inode_num; dev_t device;))
>  IF_FEATURE_FIND_USER(   ACTS(user,  uid_t uid;))
> +IF_FEATURE_FIND_NOUSER( ACTS(nouser))
> +IF_FEATURE_FIND_NOUSER( ACTS(nogroup))

Typo. (Should be IF_FEATURE_FIND_NOGROUP)

>  IF_FEATURE_FIND_SIZE(   ACTS(size,  char size_char; off_t size;))
>  IF_FEATURE_FIND_CONTEXT(ACTS(context, security_context_t context;))
>  IF_FEATURE_FIND_PAREN(  ACTS(paren, action ***subexpr;))
> @@ -891,6 +909,18 @@ ACTF(group)
> return (statbuf->st_gid == ap->gid);
>  }
>  #endif
> +#if ENABLE_FEATURE_FIND_NOUSER
> +ACTF(nouser)
> +{
> +   return !getpwuid(statbuf->st_uid);
> +}

I think there is a logic hole here.
getpwuid may return a NULL pointer on an error that's not "UID not found in
database".
Although your logic written like this conforms to POSIX, I don't know
whether in practice this would bring in security risk.

> +#endif
> +#if ENABLE_FEATURE_FIND_NOGROUP
> +ACTF(nogroup)
> +{
> +   return !getgrgid(statbuf->st_gid);
> +}

Same problem as above (getgrgid may return NULL on an error other than "not
found")

> +#endif
>  #if ENABLE_FEATURE_FIND_PRINT0
>  ACTF(print0)
>  {
> @@ -1144,6 +1174,8 @@ static action*** parse_params(char **argv)
> IF_FEATURE_FIND_QUIT(   PARM_quit  ,)
> IF_FEATURE_FIND_DELETE( PARM_delete,)
> IF_FEATURE_FIND_EMPTY(  PARM_empty ,)
> +   IF_FEATURE_FIND_NOUSER( PARM_nouser,)
> +   IF_FEATURE_FIND_NOGROUP(PARM_nogroup   ,)
> IF_FEATURE_FIND_EXEC(   PARM_exec  ,)
> IF_FEATURE_FIND_EXEC_OK(PARM_ok,)
> IF_FEATURE_FIND_EXECUTABLE(PARM_executable,)
> @@ -1196,6 +1228,8 @@ static action*** parse_params(char **argv)
> IF_FEATURE_FIND_QUIT(   "-quit\0"  )
> IF_FEATURE_FIND_DELETE( "-delete\0" )
>

Re: [PATCH v2 2/9] loop:refactor: extract subfunction get_next_free_loop()

[Kang-Che Sung]

On Mon, Nov 21, 2022 at 9:19 AM Xiaoming Ni  wrote:
> static int get_next_free_loop(char *dev, size_t dev_size, int id)
> {
>  int loopdevno = get_free_loop();
>  if (loopdevno >= 0) {
>  snprintf(dev, dev_size, LOOP_FORMAT, loopdevno);
>  return 1; /* use /dev/loop-control */
>  }
>  if (loopdevno == -2) {
>  snprintf(dev, dev_size, LOOP_FORMAT, id);
>  return 2;
>  }
>  return -1; /* no free loop devices */
> }
>
> If the dev_size parameter is added to get_next_free_loop(), the code
> size increases, Is it worth?
>
> function old new   delta
> set_loop 734 744 +10
> --
> (add/remove: 0/0 grow/shrink: 1/0 up/down: 10/0)   Total: 10
> bytes

No, that isn't what I mean. sprintf() is faster than snprintf() when
we are sure the string buffer would never overflow.
Just keep using sprintf() here but add a statement before it:
`assert(dev_size >= LOOP_NAMESIZE);`
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2 6/9] loop:refactor: Use a structure to reduce parameters

[Kang-Che Sung]

On Mon, Nov 21, 2022 at 9:31 AM Xiaoming Ni  wrote:
> > Also, it is unclear why there is the need to clone the loopinfo buffer.
> >
> >  > /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
> >  > /* (this code path is not tested) */
> >  > -   loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> >  > -   rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> >  > +   loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> >  > +   rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> >  > }
> >  > if (rc == 0) {
> >  > return lfd;
> ...
>

Pardon for my ignorance, but does the LOOP_SET_STATUS64 ioctl modify
the `loopinfo` object internally?
If the answer is yes, then it might not be a good idea to pass the
`loopinfo` structure to set_loop_configure().
I think it might be better to create the object on the fly (i.e. drop
this patch).
Otherwise, let set_loop_configure pass in a `const bb_loop_info *`
object, when we are sure it would never be modified internally.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2 9/9] loop: Add LOOP_CONFIGURE ioctl

[Kang-Che Sung]

On Friday, November 18, 2022, Xiaoming Ni  wrote:
> LOOP_CONFIGURE is added to Linux 5.8
>
> This allows userspace to completely setup a loop device with a single
> ioctl, removing the in-between state where the device can be partially
> configured - eg the loop device has a backing file associated with it,
> but is reading from the wrong offset.
>
> https://lwn.net/Articles/820408/
>
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
>
> kernel version >= 5.8, choice CONFIG_LOOP_CONFIGURE
> function old new
 delta
> set_loop 716 832
  +116
>
--
> (add/remove: 0/0 grow/shrink: 1/0 up/down: 116/0)
 Total: 116 bytes
> kernel version < 5.8, choice CONFIG_NO_LOOP_CONFIGURE
> function old new
 delta
> set_loop 760 716
   -44
>
--
> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-44)
 Total: -44 bytes
> kernel version is unknown, choice CONFIG_TRY_LOOP_CONFIGURE
> function old new
 delta
>
--
> (add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0)
 Total: 0 bytes
>
> Signed-off-by: Xiaoming Ni 
> ---
>  libbb/Config.src | 22 ++
>  libbb/loop.c | 47 ++-
>  2 files changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/libbb/Config.src b/libbb/Config.src
> index 66a3ffa23..b7f9ddab4 100644
> --- a/libbb/Config.src
> +++ b/libbb/Config.src
> @@ -369,3 +369,25 @@ config UNICODE_PRESERVE_BROKEN
> For example, this means that entering 'l', 's', ' ', 0xff, [Enter]
> at shell prompt will list file named 0xff (single char name
> with char value 255), not file named '?'.
> +
> +choice
> +   prompt "LOOP_CONFIGURE or LOOP_SET_FD + LOOP_SET_STATUS"
> +   default TRY_LOOP_CONFIGURE
> +   help
> +   LOOP_CONFIGURE is added to Linux 5.8
> +   https://lwn.net/Articles/820408/
> +   This allows userspace to completely setup a loop device with a
single
> +   ioctl, removing the in-between state where the device can be
partially
> +   configured - eg the loop device has a backing file associated
with it,
> +   but is reading from the wrong offset.
> +
> +config LOOP_CONFIGURE
> +   bool "always uses LOOP_CONFIGURE, kernel version >= 5.8"
> +
> +config NO_LOOP_CONFIGURE
> +   bool "never uses LOOP_CONFIGURE, kernel version < 5.8"
> +
> +config TRY_LOOP_CONFIGURE
> +   bool "try LOOP_CONFIGURE, kernel version is unknown"
> +
> +endchoice
> diff --git a/libbb/loop.c b/libbb/loop.c
> index ddb92fce3..be592bc4b 100644
> --- a/libbb/loop.c
> +++ b/libbb/loop.c
> @@ -126,7 +126,8 @@ static int open_file(const char *file, unsigned
flags, int *mode)
> return ffd;
>  }
>
> -static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
> +#ifndef CONFIG_LOOP_CONFIGURE
> +static int set_loop_configure_old(int ffd, int lfd, bb_loop_info
*loopinfo)
>  {
> int rc;
> bb_loop_info loopinfo2;
> @@ -149,6 +150,46 @@ static int set_loop_configure(int ffd, int lfd,
bb_loop_info *loopinfo)
> ioctl(lfd, LOOP_CLR_FD, 0); // actually, 0 param is unnecessary
> return -1;
>  }
> +#endif
> +
> +#ifndef CONFIG_NO_LOOP_CONFIGURE
> +
> +#ifndef LOOP_CONFIGURE
> +#define LOOP_CONFIGURE 0x4C0A
> +struct loop_config {
> +   uint32_t fd;
> +   uint32_t block_size;
> +   struct loop_info64 info;
> +   uint64_t __reserved[8];
> +};
> +#endif
> +
> +/*
> + * linux v5.8.0
> + * loop: Add LOOP_CONFIGURE ioctl
> + * https://lwn.net/Articles/820408/
> + *
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
> + */
> +static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
> +{
> +   int rc;
> +   struct loop_config config;
> +
> +   memset(, 0, sizeof(config));
> +   config.fd = ffd;
> +   memcpy(, loopinfo, sizeof(config.info));
> +
> +   rc = ioctl(lfd, LOOP_CONFIGURE, );
> +   if (rc == 0) {
> +   return lfd;
> +   }
> +#ifdef CONFIG_TRY_LOOP_CONFIGURE
> +   if (rc == -1 && errno == EINVAL) /* The system may not support
LOOP_CONFIGURE. */
> +   return set_loop_configure_old(ffd, lfd, loopinfo);
> +#endif

How about putting the new code into a function named
try_loop_configure_ioctl(), and let the original set_loop_configure() call
it?
This way you won't rename the function to set_loop_configure_old, the name
I personally think is ugly.



> +   return -1;
> +}
> +#endif
>
>  static int 

Re: [PATCH v2 6/9] loop:refactor: Use a structure to reduce parameters

[Kang-Che Sung]

On Friday, November 18, 2022, Xiaoming Ni  wrote:
> Step 6 of micro-refactoring the set_loop():
> Use structs to avoid transferring a large number of parameters
> in set_loop_configure() and set_loop_info()
>
> function old new   delta
> set_loop 708 720 +12
>
--
> (add/remove: 0/0 grow/shrink: 1/0 up/down: 12/0)   Total: 12
bytes
>
> Signed-off-by: Xiaoming Ni 
> ---
>  libbb/loop.c | 54 +---
>  1 file changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/libbb/loop.c b/libbb/loop.c
> index 914af57b2..2200ccb9a 100644
> --- a/libbb/loop.c
> +++ b/libbb/loop.c
> @@ -126,32 +126,21 @@ static int open_file(const char *file, unsigned
flags, int *mode)
> return ffd;
>  }
>
> -static int set_loop_configure(int ffd, int lfd, const char *file,
> -   unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
> +static int set_loop_configure(int ffd, int lfd, bb_loop_info *loopinfo)
>  {
> int rc;
> -   bb_loop_info loopinfo;
> +   bb_loop_info loopinfo2;
> /* Associate free loop device with file */
> if (ioctl(lfd, LOOP_SET_FD, ffd)) {
> return -1;
> }
> -   memset(, 0, sizeof(loopinfo));
> -   safe_strncpy((char *)loopinfo.lo_file_name, file, LO_NAME_SIZE);
> -   loopinfo.lo_offset = offset;
> -   loopinfo.lo_sizelimit = sizelimit;
> -   /*
> -* Used by mount to set LO_FLAGS_AUTOCLEAR.
> -* LO_FLAGS_READ_ONLY is not set because RO is controlled by open
type of the file.
> -* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
> -* is wrong (would free the loop device!)
> -*/
> -   loopinfo.lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
> -   rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> -   if (rc != 0 && (loopinfo.lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
> +   rc = ioctl(lfd, BB_LOOP_SET_STATUS, loopinfo);
> +   if (rc != 0 && (loopinfo->lo_flags & BB_LO_FLAGS_AUTOCLEAR)) {
> +   memcpy(, loopinfo, sizeof(*loopinfo));

Just use `loopinfo2 = loopinfo;`
Also, it is unclear why there is the need to clone the loopinfo buffer.

> /* Old kernel, does not support LO_FLAGS_AUTOCLEAR? */
> /* (this code path is not tested) */
> -   loopinfo.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> -   rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> +   loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> +   rc = ioctl(lfd, BB_LOOP_SET_STATUS, );
> }
> if (rc == 0) {
> return lfd;
> @@ -161,21 +150,36 @@ static int set_loop_configure(int ffd, int lfd,
const char *file,
> return -1;
>  }
>
> -static int set_loop_info(int ffd, int lfd, const char *file,
> -   unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
> +static int set_loop_info(int ffd, int lfd, bb_loop_info *loopinfo)
>  {
> int rc;
> -   bb_loop_info loopinfo;
> +   bb_loop_info tmp;
>
> -   rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
> +   rc = ioctl(lfd, BB_LOOP_GET_STATUS, );
>
> /* If device is free, try to claim it */
> if (rc && errno == ENXIO) {
> -   return set_loop_configure(ffd, lfd, file, offset,
sizelimit, flags);
> +   return set_loop_configure(ffd, lfd, loopinfo);
> }
> return -1;
>  }
>
> +static void init_bb_loop_info(bb_loop_info *loopinfo, const char *file,
> +   unsigned long long offset, unsigned long long sizelimit,
unsigned flags)
> +{
> +   memset(loopinfo, 0, sizeof(*loopinfo));

Would it reduce code size by doing the initialization like this?

```
   bb_loop_info empty = {0};
   loopinfo = empty;
```

> +   safe_strncpy((char *)loopinfo->lo_file_name, file, LO_NAME_SIZE);
> +   loopinfo->lo_offset = offset;
> +   loopinfo->lo_sizelimit = sizelimit;
> +   /*
> +* Used by mount to set LO_FLAGS_AUTOCLEAR.
> +* LO_FLAGS_READ_ONLY is not set because RO is controlled by open
type of the file.
> +* Note that closing LO_FLAGS_AUTOCLEARed lfd before mount
> +* is wrong (would free the loop device!)
> +*/
> +   loopinfo->lo_flags = (flags & ~BB_LO_FLAGS_READ_ONLY);
> +}
> +
>  /* Returns opened fd to the loop device, <0 on error.
>   * *device is loop device to use, or if *device==NULL finds a loop
device to
>   * mount it on and sets *device to a strdup of that loop device name.
> @@ -187,12 +191,14 @@ int FAST_FUNC set_loop(char **device, const char
*file, unsigned long long offse
> char *try;
> struct stat statbuf;
> int i, lfd, ffd, mode, rc;
> +   bb_loop_info loopinfo;
>
> ffd = open_file(file, flags, );
>  

Re: [PATCH v2 2/9] loop:refactor: extract subfunction get_next_free_loop()

[Kang-Che Sung]

On Friday, November 18, 2022, Xiaoming Ni  wrote:
> Step 2 of micro-refactoring the set_loop function ()
> Extract subfunction get_next_free_loop() from set_loop()
>
> Also fix miss free(try) when stat(try) and mknod fail
>
> function old new   delta
> set_loop 758 734 -24
>
--
> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-24) Total: -24
bytes
>
> Fixes: 3448914e8cc5 ("mount,losetup: use /dev/loop-control is it exists")
> Signed-off-by: Xiaoming Ni 
> ---
>  libbb/loop.c | 55 
>  1 file changed, 25 insertions(+), 30 deletions(-)
>
> diff --git a/libbb/loop.c b/libbb/loop.c
> index c517ceb13..71fd8c1bc 100644
> --- a/libbb/loop.c
> +++ b/libbb/loop.c
> @@ -96,6 +96,20 @@ int FAST_FUNC get_free_loop(void)
> return loopdevno; /* can be -1 if error */
>  }
>
> +static int get_next_free_loop(char *dev, int id)
> +{
> +   int i = get_free_loop();
> +   if (i >= 0) {
> +   sprintf(dev, LOOP_FORMAT, i);
> +   return 1; /* use /dev/loop-control */
> +   } else if (i == -2) {
> +   sprintf(dev, LOOP_FORMAT, id);
> +   return 2;
> +   } else {
> +   return -1; /* no free loop devices */
> +   }
> +}

I'm a little nervous when the buffer length of `dev` is not passed into
this function. Yes I know the buffer is large enough for the loop device
path that would be printed. But I just wish there would be an assertion in
this function, so that if the function is reused somewhere else, the
developer would know what he is doing.

> +
>  static int open_file(const char *file, unsigned flags, int *mode)
>  {
> int ffd;
> @@ -132,30 +146,26 @@ int FAST_FUNC set_loop(char **device, const char
*file, unsigned long long offse
>
> try = *device;
> if (!try) {
> - get_free_loopN:
> -   i = get_free_loop();
> -   if (i == -1) {
> -   close(ffd);
> -   return -1; /* no free loop devices */
> -   }
> -   if (i >= 0) {
> -   try = xasprintf(LOOP_FORMAT, i);
> -   goto open_lfd;
> -   }
> -   /* i == -2: no /dev/loop-control. Do an old-style search
for a free device */
> try = dev;
> }
>
> /* Find a loop device */
> /* 0xf is a max possible minor number in Linux circa 2010 */
> for (i = 0; i <= 0xf; i++) {
> -   sprintf(dev, LOOP_FORMAT, i);
> +   if (!*device) {
> +   rc = get_next_free_loop(dev, i);
> +   if (rc == -1) {
> +   break; /* no free loop devices */
> +   } else if (rc == 1) {
> +   goto open_lfd;
> +   }
> +   }
>
> IF_FEATURE_MOUNT_LOOP_CREATE(errno = 0;)
> if (stat(try, ) != 0 ||
!S_ISBLK(statbuf.st_mode)) {
> if (ENABLE_FEATURE_MOUNT_LOOP_CREATE
>  && errno == ENOENT
> -&& try == dev
> +&& (!*device)
> ) {
> /* Node doesn't exist, try to create it */
> if (mknod(dev, S_IFBLK|0644, makedev(7,
i)) == 0)
> @@ -188,13 +198,10 @@ int FAST_FUNC set_loop(char **device, const char
*file, unsigned long long offse
> /* Associate free loop device with file */
> if (ioctl(lfd, LOOP_SET_FD, ffd)) {
> /* Ouch. Are we racing with other mount?
*/
> -   if (!*device   /* yes */
> -&& try != dev /* tried a
_kernel-offered_ loopN? */
> -   ) {
> -   free(try);
> +   if (!*device) {
> close(lfd);
>  //TODO: add "if (--failcount != 0) ..."?
> -   goto get_free_loopN;
> +   continue;
> }
> goto close_and_try_next_loopN;
> }
> @@ -218,8 +225,6 @@ int FAST_FUNC set_loop(char **device, const char
*file, unsigned long long offse
> }
> if (rc == 0) {
> /* SUCCESS! */
> -   if (try != dev) /* tried a kernel-offered
free loopN? */
> -   *device = try; /* malloced */
> if (!*device)   /* was looping in search
of 

Re: [PATCH 9/9] loop: Add LOOP_CONFIGURE ioctl

[Kang-Che Sung]

On Fri, Nov 18, 2022 at 9:04 AM Xiaoming Ni  wrote:
>
> LOOP_CONFIGURE is added to Linux 5.8
>
> This allows userspace to completely setup a loop device with a single
> ioctl, removing the in-between state where the device can be partially
> configured - eg the loop device has a backing file associated with it,
> but is reading from the wrong offset.
>
> https://lwn.net/Articles/820408/
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc5
>

Hello.

1. Are the patches 1 to 8 you proposed in this set _not_ relevant to
the new LOOP_CONFIGURE ioctl?
I.e. They are general code structure improvements to the busybox loop
device code and do not add any feature?
Note that according to the bloat-o-meter results you included in the
patches, some of them increase code size, and you should explain why
the code size increases there, and why you think it is okay for the
patch to increase code size.
2. I think LOOP_CONFIGURE support can be made into a config option, so
that builders can have choice on which algorithm should be built into
their busybox binary.
* Always use LOOP_CONFIGURE ioctl, or
* Support LOOP_CONFIGURE, but keep the old code for runtime fallback, or
* Not support LOOP_CONFIGURE at all (always use the old code)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] buildsys: resurrect PLATFORM_LINUX and depend on it for linux-specific applets

[Kang-Che Sung]

On Sat, Nov 5, 2022 at 12:55 AM Michael Tokarev  wrote:

> diff --git a/Makefile b/Makefile
> index 503475fe9..ad780c261 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -361,9 +361,16 @@ scripts/basic/%: scripts_basic ;
>
>  # This target generates Kbuild's and Config.in's from *.c files
>  PHONY += gen_build_files
> -gen_build_files: $(wildcard $(srctree)/*/*.c) $(wildcard $(srctree)/*/*/*.c) 
> $(wildcard $(srctree)/embed/*)
> +gen_build_files: $(wildcard $(srctree)/*/*.c) $(wildcard $(srctree)/*/*/*.c) 
> $(wildcard $(srctree)/embed/*) .platform.in
> $(Q)$(srctree)/scripts/gen_build_files.sh $(srctree) $(objtree)
>
> +.platform.in:
> +   $(Q)printf '#ifndef __linux__\nplatform_is_not_linux\n#endif' \
> +   | $(CPP) - | grep -s platform_is_not_linux \
> + && linux=n || linux=y; \
> +   printf "config PLATFORM_LINUX\n\tbool\n\tdefault $$linux\n" > $@
> +MRPROPER_FILES += .platform.in
> +
>  # bbox: we have helpers in applets/
>  # we depend on scripts_basic, since scripts/basic/fixdep
>  # must be built before any other host prog

What is the reason for preferring #ifndef check rather than #ifdef?
I mean, that would make PLATFORM_LINUX defaults to y if the compiler fails,
and I would expect n in that case.

And I'm curious. Does Busybox's kconfig system supports macros as described in
 ?
If the system supports macros, we can put this in the Kconfig file and simplify
a lot of things:

config PLATFORM_LINUX
bool
default $(shell, { printf '#ifndef __linux__\n#error \n#endif\n' \
| $(CPP) - -o /dev/null; } >/dev/null 2>&1 && echo y || echo n)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v4] shell: exchange Dijkstra $(( )) evaluator..

[Kang-Che Sung]

On Wednesday, September 7, 2022, Steffen Nurpmeso 
wrote:
> Kang-Che Sung wrote in
>  :
>  |On Wednesday, September 7, 2022, Steffen Nurpmeso 
>  |wrote:
>  ...
>  |>|> +  if(su_64( i > U32_MAX || ) i >= UZ_MAX / 2 ||
>  ...
>  |>|I have to admit that the amount of macro maze makes it really hard to
>  |>|read ;)
>  |>
>  |> Well it is easier than having lots of #ifdef #else #endif blocks
>  |> in my opinion.  On 32-bit the above would spit out a warning
>  |> (possibly, i have not really looked what compiler/linker flags
>  |> busybox uses) because i>U32_MAX can never be true.  I mean i could
>  ...
>  |Compiler will optimize out always-false expressions without giving any
>  |warning.
>
> Regarding the warning: not if you excess an integer limit.
> If you say if(0){} i go with you.
> Hm.  Well, actually it seems current compilers really do so in
> simplemost test files at least, like
>
>   int main(void){if(10>0xULL)return 1;return 0;}
>
> But already if it is a bit more complicated there is
>
>   int main(void){
> unsigned long long int i = 10;
> if(i>0xULL)return 1;
> return 0;}
>
>   t.c:3:6: warning: result of comparison 'unsigned long long' >
18446744073709551615 is always false [-Wtautological-type-limit-compare]
>   if(i>0xULL)return 1;
>  ~^~
>   1 warning generated.
>
> I can assure you that i am surprised, "in earlier times" you would
> have been thrown warnings at.  (gcc 12.2.0 is silent!!!)

Okay, I didn't notice there is such a warning in Clang, but I would prefer
writing this way:

if ( (sizeof(i) > 4 && i > U32_MAX) || i >= UZ_MAX / 2 ...
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v4] shell: exchange Dijkstra $(( )) evaluator..

[Kang-Che Sung]

On Wednesday, September 7, 2022, Steffen Nurpmeso 
wrote:
>  |> +  /* Overflow check: since arithmetic expressions are rarely \
>  |> long enough
>  |> +   * to come near this limit, xxx laxe & fuzzy, not exact; max \
>  |> U32_MAX! */
>  |> +  if(su_64( i > U32_MAX || ) i >= UZ_MAX / 2 ||
>  |
>  |I have to admit that the amount of macro maze makes it really hard to
>  |read ;)
>
> Well it is easier than having lots of #ifdef #else #endif blocks
> in my opinion.  On 32-bit the above would spit out a warning
> (possibly, i have not really looked what compiler/linker flags
> busybox uses) because i>U32_MAX can never be true.  I mean i could
> say i>=U32_MAX-1 or what, sure.  The above is "laxe and fuzzy"
> anyhow.  :-)

Compiler will optimize out always-false expressions without giving any
warning.
It's common for C code to produce many expressions like this that evaluate
to constants (boolean or integer) at compile time as results of macro
expansion, so no warnings should be given.

The expression with a macro like `su_64( i > U32_MAX || )` looks really
ugly to me. If what you are achieving is to remove statements that are
always false (under certain preprocessor condition), just let the compiler
do the job.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ntpd: make NTP client Y2036-ready

[Kang-Che Sung]

On Monday, May 9, 2022, Miroslav Lichvar  wrote:
> The 32-bit integer part of the NTP timestamp overflows in year 2036,
> which starts the second NTP era.
>
> Modify the timestamp conversion to shift values between 1900-1970 (in
> the first era) to the second era to enable the client to synchronize
> correctly until year 2106 (assuming 64-bit time_t).
>
> Signed-off-by: Miroslav Lichvar 

Is this the right fix where there is no check on the era number?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v4] tree: new applet

[Kang-Che Sung]

Hi Roger,

May I suggest you add an option to draw the tree lines using ASCII
characters only, and make the Unicode support optional at build time?

I just feel uncomfortable when I see source code containing embedded UTF-8
characters and the strings have no ASCII alternative.

The DOS tree command has the "/a" option. You know what I mean.

Thank you.

On Sunday, May 1, 2022, Roger Knecht  wrote:
> Add new applet which resembles the MS-DOS tree program to list
directories and files in a tree structure.
>
> function old new   delta
> tree_print - 388+388
> .rodata95678   95767 +89
> tree_main  -  73 +73
> tree_print_prefix  -  28 +28
> packed_usage   34417   34429 +12
> globals-   8  +8
> applet_main 31923200  +8
> applet_names27472752  +5
>
--
> (add/remove: 5/0 grow/shrink: 4/0 up/down: 611/0) Total: 611
bytes
>
> Signed-off-by: Roger Knecht 
> ---
> Changelog:
>
> V4:
> - Rephrase commit message
> - Updated bloatcheck to latest master
>
> V3:
> - Fixed symlink handling
> - Handle multiple directories in command line arguments
> - Extended tests for symlink and multiple directories
> - Reduced size by using libbb functions
>
> V2:
> - Fixed tree help text
> - Reduced size by 644 bytes
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 2/2] less: replace most uses of NORMAL escape with UNHIGHLIGHT

[Kang-Che Sung]

On Friday, April 15, 2022, FriendlyNeighborhoodShane <
shane.880088.s...@gmail.com> wrote:
> Fixes conflict when -R's color escape codes are mixed together with
> highlights. Better complement to HIGHLIGHT.

What's the difference between the NORMAL escape and the UNHIGHLIGHT escape?
Is there a test case to demonstrate the difference?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: grep: add option for better usability in pipelines

[Kang-Che Sung]

On Thu, Feb 17, 2022 at 3:27 PM Ulrich Eckhardt
 wrote:
> >
> > Why do we need to implement a workaround in grep while you can
> > do this in shell to ignore the exit code of grep?
> >
> > { grep issue 
> `grep -p ...` rather replaces `grep ... || test $? = 1`. The advantage
> is that it is explicit about what it does. Understanding the intent of
> `-p` or `--pipe` is easier than understanding the alternative.
>

Unless you can propose the '-p' option to POSIX will busybox implement it as
you say.

The problem here is portability. FreeBSD grep
(https://www.freebsd.org/cgi/man.cgi?query=grep) has a different meaning with
'-p' so it won't be portable to use '-p' in shell scripts. And the long option
'--pipe' isn't quite helpful either as it would end up being a GNU-only
extension and so is discouraged to use in shell scripts (which defeats the
purpose of your '--pipe' option in the first place).
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: grep: add option for better usability in pipelines

[Kang-Che Sung]

On Thu, Feb 17, 2022 at 12:50 PM Michael Conrad  wrote:
>
> On 2/16/22 19:32, Kang-Che Sung wrote:
> >
> >> Now, for an example where it makes a difference. Consider a Bash script
> >> like this:
> >>
> >># enable automatic error handling
> >>set -eo pipefail
> >># check for string "issues" in a logfile
> >>cat logfile | grep issue | sort --unique
> >>
> >> If there are no issues in the logs, grep return exit code 1 and the
> >> shell interprets this as an error and exits itself.
> >>
> > Why do we need to implement a workaround in grep while you can
> > do this in shell to ignore the exit code of grep?
> >
> > { grep issue 
> In order to implement his suggestion, you need to only ignore exit code
> 1, while still allowing other exit codes to abort the script. (like a
> system error while reading the file)

{ grep issue  But also, I've never seen ":" used as a command... where is that
> documented?  Is it equivalent to 'true'?

https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.71/html_node/Limitations-of-Builtins.html
Look for the "true" section.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: grep: add option for better usability in pipelines

[Kang-Che Sung]

On Thu, Feb 17, 2022 at 8:01 AM Ulrich Eckhardt
 wrote:
>
> I've implemented a `-p` flag for grep, which disables a behaviour that
> the exit status is used (abused?) to signal whether any match
> occurred.

The behaviour is POSIX.

> Now, for an example where it makes a difference. Consider a Bash script
> like this:
>
>   # enable automatic error handling
>   set -eo pipefail
>   # check for string "issues" in a logfile
>   cat logfile | grep issue | sort --unique
>
> If there are no issues in the logs, grep return exit code 1 and the
> shell interprets this as an error and exits itself.
>

Why do we need to implement a workaround in grep while you can
do this in shell to ignore the exit code of grep?

{ grep issue  I've implemented that feature here, though it lacks tests yet:
> https://github.com/UlrichEckhardt/busybox/tree/grep-pipe-option
> Also, I'm currently trying to get the same feature into GNU grep as
> well, the long form `--pipe` is used there. I've also considered
> `--filter` (because it only filters) as alternative. I'm not fully
> happy with either one, maybe someone here comes up with a better
> suggestion.

I don't see the '--pipe' option in GNU grep manual

and I don't think it would be needed considering that you can tell the
shell to ignore an exit code already.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Patch to add support for RFC2324 (HTCPCP)

[Kang-Che Sung]

On Mon, Nov 29, 2021 at 5:16 AM Peter Willis  wrote:
>
> Hi busybox devs, It's been a long time! About 17 years since my last 
> submission :-)
>
> I was just trying to make some coffee with busybox, and I noticed it doesn't 
> support RFC 2324 (Hyper Text Coffee Pot Control Protocol). Attached is a 
> patch that adds support for the standard. Although I should mention it's not 
> full support for the standard; I take my coffee black, so I didn't implement 
> WHEN and Accept-Additions, but I'm sure someone else can if they need creamer 
> (although some Kahlua wouldn't go amiss with this winter weather...)
>
> The patch includes a configuration file option "T" that sets if the host is a 
> teapot or not. The default is teapot mode, for portability (coffee brewing 
> operations shouldn't happen on a teapot).
>
> Sample operation:
>
> $ echo "T:1" > cgi-bin/httpd.conf
> $ curl -d 'start' -H "Content-Type: application/coffee-pot-command" -X BREW 
> http://localhost:6789/cgi-bin/coffeepot
> 418 I'm a teapot
> 418 I'm a teapot
> The web server is a teapot
> 
> $ echo "T:0" > cgi-bin/httpd.conf
> $ curl -d 'start' -H "Content-Type: application/coffee-pot-command" -X BREW 
> http://localhost:6789/cgi-bin/coffeepot
> Brewing coffee!
>
> Also note that the patch fixes a Content-Length bug I found in send_headers():
>
> The function always returns the Content-Length, which is always set to 
> the length of a file (for example, if there was a request of a file, the 
> file's size is taken - but then some error might be thrown after this point). 
> After the Content-Length is set, if infoString was set (the text of a 
> response code) the resulting HTML output's length bears no relation to the 
> file size it previously set as the Content-Length. Therefore the 
> Content-Length needs to be set to either the file size, or the length of the 
> infoString HTML message. The patch includes a change to calculate the size of 
> the infoString template and return that length if infoString was set.

Just wondering. Is this an out-of-season April Fools joke? (Sorry for
quoting that guy at Blizzcon.)

But I have a serious question here: I don't see the ".cup" file
extension or the "vessel/cup" MIME type defined anywhere in the HTCPCP
spec. Where are those keywords from?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ed: add support for -p command-line option as mandated by POSIX

[Kang-Che Sung]

On Sunday, November 21, 2021, Harald van Dijk  wrote:
> On 21/11/2021 07:12, Kang-Che Sung wrote:
>>
>> On Sunday, November 21, 2021, David Laight mailto:david.lai...@aculab.com>> wrote:
>>  >
>>  > You shouldn't need the strdup().
>>  > I think you can even do:
>>  > if (!(opt & 1))
>>  > prompt = "";
>>  > because (IIRC and for historic reasons) quoted strings are char[] not
const char[].
>>
>> I don't know why you are messing up with the "constness" of the strings.
C standard says string literal is of type const char[], and the const
keyword didn't exist before C89.
>> Note the compiler is free to merge string literals with identical
content so they share the same buffer in the .rodata section (that's why
they are const).
>
> It does not say that, it says the opposite:
>
>   The multibyte character sequence is then used to initialize an array
>   of static storage duration and length just sufficient to contain the
>   sequence. For character string literals, the array elements have type
>   char, and are initialized with the individual bytes of the multibyte
>   character sequence.
>
> Note the "char" as opposed to "const char".

It refers to the initialization like this:

   char str[] = "abc";
   str[0] = 'd'; // Vaild

not this:

   char arr[5];
   arr = "abc";
   arr[0] = 'd'; // Undefined behavior

nor this:

   char *ptr;
   ptr = "abc";
   *ptr = 'd'; // Undefined behavior

And messing with the "constness" is always a bad idea.

The compiler actually treats the string literals of the latter two examples
to have the type 'const char[]', so modifying the contents yields undefined
behavior, matching what the standard says.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ed: add support for -p command-line option as mandated by POSIX

[Kang-Che Sung]

On Sunday, November 21, 2021, David Laight  wrote:
> From: soe...@soeren-tempel.net
>> Sent: 20 November 2021 17:17
>>
>> The POSIX.1-2008 specification of ed(1) mandates two command-line
>> options: -p (for specifying a prompt string) and -s (to suppress writing
>> of byte counts). This commit adds support for the former. Furthermore,
>> it also changes the default prompt string to an empty string (instead
>> of ": ") since this is also mandated by POSIX:
>>
>>   -p string Use string as the prompt string when in command mode.
>> By default, there shall be no prompt string.
>>
> ...
>> - if (argv[1]) {
>> - fileName = xstrdup(argv[1]);
>> + opt = getopt32(argv, "p:", );
>> + if (!(opt & 0x1))
>> + prompt = xstrdup(""); /* no prompt by default */
>
> You shouldn't need the strdup().
> I think you can even do:
> if (!(opt & 1))
> prompt = "";
> because (IIRC and for historic reasons) quoted strings are char[] not
const char[].

I don't know why you are messing up with the "constness" of the strings. C
standard says string literal is of type const char[], and the const keyword
didn't exist before C89.
Note the compiler is free to merge string literals with identical content
so they share the same buffer in the .rodata section (that's why they are
const).
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: lzip support in busybox?

[Kang-Che Sung]

On Wed, Aug 25, 2021 at 1:32 AM Jacob Burkholder
 wrote:
>
> In 2012 a patch was submitted to the list about lzip support in busybox, 
> there was not much discussion about it.  I've been using the patch for some 
> years and keep porting it to new versions of busybox, just now 1.33.1.  I use 
> a busybox based linux distribution as a build environment and the patch is 
> useful as more projects distribute their source distribution as .tar.lz.  Any 
> chance to get the latest patch committed?  I can submit the patch applied to 
> busybox 1.33.1 or 1.34.0.

If I remember correctly, the original lzip patch uses a separate LZMA
decompression code, which duplicates a lot of what xz (LZMA2)
decompression would do.
It would be better if lzip and xz share code as much as possible. It
would be bad to maintain two versions of the code for algorithms that
are mostly the same.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: add built-in $ASH_VERSION variable

[Kang-Che Sung]

On Thu, Mar 11, 2021 at 2:55 PM Ariadne Conill  wrote:
>
> This is helpful for detecting if the shell is busybox ash or not,
> which is necessary for enabling ash-specific features in /etc/profile
> and Alpine's default $ENV.
>
> https://gitlab.alpinelinux.org/alpine/aports/-/issues/12398 outlines
> the rationale for detecting what shell is running in /etc/profile and
> similar.
>
> function old new   delta
> varinit_data 360 384 +24
> .rodata77899   77922 +23
> bbconfig_config_bz2 61276120  -7
> --
> (add/remove: 0/0 grow/shrink: 2/1 up/down: 47/-7)  Total: 40 bytes
>
> Signed-off-by: Ariadne Conill 

Setting ASH_VERSION to BusyBox's version seems to be a bad idea.
There are many variants of ash shell and BusyBox's is just one of them.
A quick Google search also shows that ASH_VERSION (as an env variable) is used
by someone else. The variable name is not exclusive to us.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: new option FEATURE_WGET_FTP to enable/disable FTP

[Kang-Che Sung]

On Monday, January 18, 2021, Sergey Ponomarev  wrote:
> Introduce a separate option FTPS_SUPPORTED instead of not obvious
ENABLE_FEATURE_WGET_HTTPS.
>

I juat wonder...
If you are bothered to add FTP configure option, how about making FTPS
separate from HTTPS as well?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2 1/2] modutils: check ELF header before calling finit_module()

[Kang-Che Sung]

On Mon, Jan 4, 2021 at 6:32 PM Qu Wenruo  wrote:
>
> On 2021/1/4 下午6:01, Kang-Che Sung wrote:
> > On Sun, Jan 3, 2021 at 12:11 PM Qu Wenruo  wrote:
> >>
> >> finit_module() and init_module() system calls have clear specification
> >> to only accept valid ELF image.
> >>
> >> Although we try finit_module() on compressed modules to let the kernel
> >> determine if it's an ELF image, but it's not ideal, especially when
> >> newer kernel will complain when some invalid files/memory is passed in.
> >>
> >> Treat the kernel better by just doing a very basic ELF header check
> >> before calling finit_module().
> >>
> >> Signed-off-by: Qu Wenruo 
> >
> > What is the reason for not letting the kernel do all the ELF sanity checks?
> > Performance? Security? For now this looks like extra code to busybox
> > without obvious benefits.
> >
> To avoid possible "Invalid ELF header" error message from kernel.
>
> Since those system calls are only to accept ELF header, kernel has its
> right to info the caller that it got some invalid ELF header (even if
> it's just compressed file).
>
> Or did you mean, busybox pursues size so much that it doesn't matter to
> not follow system call spec?

It is normal for the kernel to receive a malformed ELF file through
init_module() and it's the kernel's job to reject it. I don't see why the
"Invalid ELF header" message would bother you so much, since you
won't load kernel modules often.

By "security" I mean, if the kernel would accept any header other than
ELF and you want to ensure only ELF is passed to the system call,
then it's fine to add that sanity check. Otherwise, there's no benefit
for repeating what the kernel would do in busybox.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v2 1/2] modutils: check ELF header before calling finit_module()

[Kang-Che Sung]

On Sun, Jan 3, 2021 at 12:11 PM Qu Wenruo  wrote:
>
> finit_module() and init_module() system calls have clear specification
> to only accept valid ELF image.
>
> Although we try finit_module() on compressed modules to let the kernel
> determine if it's an ELF image, but it's not ideal, especially when
> newer kernel will complain when some invalid files/memory is passed in.
>
> Treat the kernel better by just doing a very basic ELF header check
> before calling finit_module().
>
> Signed-off-by: Qu Wenruo 

What is the reason for not letting the kernel do all the ELF sanity checks?
Performance? Security? For now this looks like extra code to busybox
without obvious benefits.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] shrink last_char_is function even more

[Kang-Che Sung]

On Monday, July 20, 2020, Tito  wrote:
>
>
> On 7/20/20 9:22 AM, Laurent Bercot wrote:
>>> The param should be marked with the nonnull attribute, just like the
>>> libc string functions. Then the compiler will warn you if you try to
>>> pass NULL (may need higher optimization, warning levels, or the
>>> analyzer mode in complex cases).
>>
>>  Indeed.
>>
>>  A function that takes a pointer that *cannot* be NULL, and a function
>> that takes a pointer that *may* be NULL, are not the same thing at all.
>>  This is one of the main reasons while a lot of people find C pointers
>> difficult: a pointer can be used for two very different things, namely
>> unconditionally representing an object (passing it by address), or
>> representing the *possibility* of an object. In ocaml, the former would
>> would be typed "'a ref", and the latter "'a ref option", and those are
>> *not the same type*.
>>
>>  When writing and using a function that takes pointers, a C programmer
>> should always be very aware of the kind of pointer the function expects.
>> It is a programming error to pass NULL to a function expecting a pointer
>> that cannot be NULL, and that error should be caught as early as
>> possible. The nonnull attribute helps detect it at compile time. And
>> at run time, if the function gets NULL, it should crash, as loudly as
>> possible, in order for the bug to be fixed.
>
> Hi,
>
> while I agree with you in theory, the reality looks different to
> me, being an external observer, because with increasing code base,
> increasing complexity, increasing number of people messing with
> the code, increasing time pressure the result is a flood of CVE's
> and updates and patch Tuesdays, etc..
> So a question arises to me, aren't all this programmers able to do their
job or
> is this the result of sloppy programming or a deficiency of defensive
> programming taking into account that humans for all the aforementioned
> reasons will certainly do mistakes in a unpredictable but very effective
way?
> I for myself decided for being defensive and this until today
> payed off for my little personal codebase.
> I save you from showing examples of my defensive coding
> that I'm sure would horrify most of the list members ;-)
>

Hi. Just my small opinion here.

Coding defensively is not necessarily a wrong thing by itself. And I do
write things defensively for my programming job.

The key point is that coding defensively does not mean adding unnecessary,
sanity checks for everything. That would hurt performance. When a function
expects a non-null pointer argument from the caller (and you are sure that
caller code is sane already), use GCC's nonnull attribute and assertions.
That's what assert statements are for - conveying your code assumptions and
do the sanity checks for debugging and only in the debug build. That would
keep your "defensive" attitude without sacrificing any performance thing.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] libbb/last_char_is: rewrite for smaller and faster code

[Kang-Che Sung]

On Saturday, July 4, 2020, Harald van Dijk  wrote:
>
> This is really the same problem as with standard library functions such
as strchr(). The function should have a return type of char* if its
argument has type char*. The function should have a return type of const
char* if its argument has type const char*. The current signature is the
only signature that allows all code that should be valid, at the expense of
not rejecting code that should be invalid.

Just to give a note: The const-correctness problem could have been avoided
if the function API returns an offset (of `size_t` type) rather than the
pointer directly. But it may be too late for the standard library to
correct this historical mistake.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] libbb/last_char_is: rewrite for smaller and faster code

[Kang-Che Sung]

On Friday, July 3, 2020, Jody Bruchon  wrote:
>
>
> On July 2, 2020 11:29:06 AM EDT, Kang-Che Sung 
wrote:
>>On Thursday, July 2, 2020, Jody Bruchon  wrote:
>>>  /* Find out if the last character of a string matches the one given
>>*/
>>> -char* FAST_FUNC last_char_is(const char *s, int c)
>>> +char* FAST_FUNC last_char_is(char *s, char c)
>>
>>Why are you removing the const qualifier, and how would that reduce the
>>code size?
>
> Why does it need the const in the first place? The code I wrote does
reduce the code size.

Any pointer parameter whose content is meant to be read-only is good to be
declared const. It's a compiler safety check that you won't accidentally
modify the content there. The const qualifier by itself doesn't add code.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] libbb/last_char_is: rewrite for smaller and faster code

[Kang-Che Sung]

On Thursday, July 2, 2020, Jody Bruchon  wrote:
>  /* Find out if the last character of a string matches the one given */
> -char* FAST_FUNC last_char_is(const char *s, int c)
> +char* FAST_FUNC last_char_is(char *s, char c)

Why are you removing the const qualifier, and how would that reduce the
code size?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Invalid tar magic when streaming download

[Kang-Che Sung]

On Fri, Dec 6, 2019 at 10:06 AM Jeffrey Fetterman
 wrote:
>
> Ah. I don't think this mailing list is for me. Thank you for your time.
>

What are you expecting anyway?

1. It is normal for a tar program to reject a tarball with multiple layers of
gzip. Some tar implementations allows recurse decompression, but that's an
exception, not a rule. (They should have a recursion limit, by the way, or else
they can't defend against "gzip" bombs.)
2. For the .zip file, you should use "unzip", which is available in BusyBox if
you build it. "tar" is a wrong program for that.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Invalid tar magic when streaming download

[Kang-Che Sung]

On Friday, December 6, 2019, Eli Schwartz  wrote:
> libarchive bsdtar works, which I guess means that libarchive permits you
> to wrap a tarball in *two* layers of gzip compression, then extract the
> contents. Personally, I would claim this is a buggy design goal, because
> you'd have to be nuts to create tarballs with multiple layers of
> recursive compression.
>

I think it's also good to mention that allowing extraction of multiple
layers of compression could lead to a security risk (DoS) for tar.

There are specifically crafted gzip archives on the Internet meant to crash
(or stress) antivirus scanners with multiple layers of compression. And
there is also a thing called "gzip quine". I think it's a right choice for
tar to reject it at first. If users wants decompressing multiple layers,
they can do it with a shell script loop.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[RFC] gzip: Default compress level changeable at build time.

[Kang-Che Sung]

This introduces GZIP_DEFAULT_LEVEL macro so that builders can easily
change it by overriding (e.g. "-DGZIP_DEFAULT_LEVEL=8").

Currently, this patch is ugly. I apologize. (Because I utilize a lot
of preprocessor tricks in order to implement a build-time constant
table lookup.)

This is how I proposed letting the user change the compression level
when GZIP_LEVELS is turned off, when discussing about gzip default
compression level. This patch is a proof of concept of that.
<http://lists.busybox.net/pipermail/busybox/2019-September/087442.html>

Signed-off-by: Kang-Che Sung 
---
 archival/gzip.c | 50 -
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/archival/gzip.c b/archival/gzip.c
index 3966a06b4..10cf2f884 100644
--- a/archival/gzip.c
+++ b/archival/gzip.c
@@ -251,6 +251,17 @@ typedef unsigned IPos;
  * save space in the various tables. IPos is used only for parameter passing.
  */
 
+#define LEVEL_CONFIG_LIST \
+   /* (level, good, chain_shift, lazy, nice) */ \
+   LEVEL_CONFIG_DEFINE(4,4,   4,4,   16) \
+   LEVEL_CONFIG_DEFINE(5,8,   5,   16,   32) \
+   LEVEL_CONFIG_DEFINE(6,8,   7,   16,  128) \
+   LEVEL_CONFIG_DEFINE(7,8,   8,   32,  128) \
+   LEVEL_CONFIG_DEFINE(8,   32,  10,  128,  258) \
+   LEVEL_CONFIG_DEFINE(9,   32,  12,  258,  258)
+
+#define GZIP_DEFAULT_LEVEL 6
+
 enum {
WINDOW_SIZE = 2 * WSIZE,
 /* window size, 2*WSIZE except for MMAP or BIG_MEM, where it is the
@@ -258,14 +269,25 @@ enum {
  */
 
 #if !ENABLE_FEATURE_GZIP_LEVELS
+# define LEVEL_CONFIG_DEFINE(level, good, chain_shift, lazy, nice) \
+   l##level##_chain = (1 << (chain_shift)), \
+   l##level##_lazy = (lazy), \
+   l##level##_good = (good), \
+   l##level##_nice = (nice),
+
+   LEVEL_CONFIG_LIST
+
+# define LEVEL_CONFIG_(level, suffix) l##level##suffix
+# define LEVEL_CONFIG(level, name) LEVEL_CONFIG_(level, _##name)
+
+   comp_level_minus4 = (GZIP_DEFAULT_LEVEL - 4),
 
-   comp_level_minus4 = 6 - 4,
-   max_chain_length = 128,
+   max_chain_length = LEVEL_CONFIG(GZIP_DEFAULT_LEVEL, chain),
 /* To speed up deflation, hash chains are never searched beyond this length.
  * A higher limit improves compression ratio but degrades the speed.
  */
 
-   max_lazy_match = 16,
+   max_lazy_match = LEVEL_CONFIG(GZIP_DEFAULT_LEVEL, lazy),
 /* Attempt to find a better match only when the current match is strictly
  * smaller than this value. This mechanism is used only for compression
  * levels >= 4.
@@ -277,7 +299,7 @@ enum {
  * max_insert_length is used only for compression levels <= 3.
  */
 
-   good_match = 8,
+   good_match = LEVEL_CONFIG(GZIP_DEFAULT_LEVEL, good),
 /* Use a faster search when the previous match is longer than this */
 
 /* Values for max_lazy_match, good_match and max_chain_length, depending on
@@ -286,12 +308,16 @@ enum {
  * found for specific files.
  */
 
-   nice_match = 128,   /* Stop searching when current match exceeds 
this */
+   nice_match = LEVEL_CONFIG(GZIP_DEFAULT_LEVEL, nice)
+/* Stop searching when current match exceeds this */
 /* Note: the deflate() code requires max_lazy >= MIN_MATCH and max_chain >= 4
  * For deflate_fast() (levels <= 3) good is ignored and lazy has a different
  * meaning.
  */
-#endif /* ENABLE_FEATURE_GZIP_LEVELS */
+# undef LEVEL_CONFIG_DEFINE
+# undef LEVEL_CONFIG_
+# undef LEVEL_CONFIG
+#endif /* !ENABLE_FEATURE_GZIP_LEVELS */
 };
 
 struct globals {
@@ -2204,12 +2230,10 @@ int gzip_main(int argc UNUSED_PARAM, char **argv)
uint8_t lazy2;
uint8_t nice2;
} gzip_level_config[6] = {
-   {4,   4,   4/2,  16/2}, /* Level 4 */
-   {8,   5,  16/2,  32/2}, /* Level 5 */
-   {8,   7,  16/2, 128/2}, /* Level 6 */
-   {8,   8,  32/2, 128/2}, /* Level 7 */
-   {32, 10, 128/2, 258/2}, /* Level 8 */
-   {32, 12, 258/2, 258/2}, /* Level 9 */
+# define LEVEL_CONFIG_DEFINE(level, good, chain_shift, lazy, nice) \
+   {(good), (chain_shift), (lazy)/2, (nice)/2},
+   LEVEL_CONFIG_LIST
+# undef LEVEL_CONFIG_DEFINE
};
 #endif
 
@@ -2229,7 +2253,7 @@ int gzip_main(int argc UNUSED_PARAM, char **argv)
 #if ENABLE_FEATURE_GZIP_LEVELS
opt >>= (BBUNPK_OPTSTRLEN IF_FEATURE_GZIP_DECOMPRESS(+ 2) + 1); /* drop 
cfkvq[dt]n bits */
if (opt == 0)
-   opt = 1 << 5; /* default: 6 */
+   opt = 1 << (GZIP_DEFAULT_LEVEL - 1);
opt = ffs(opt >> 4); /* Maps -1..-4 to [0], -5 to [1] ... -9 to [5] */
 
comp_level_minus4 = opt;
-- 
2.18.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] bc: Add 'U' suffix in UINT_MAX preprocessor check

[Kang-Che Sung]

Without the 'U' unsigned suffix, gcc will throw a "integer constant is
so large that it is unsigned" warning.
---
 miscutils/bc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/miscutils/bc.c b/miscutils/bc.c
index e492f0f50..92721d18f 100644
--- a/miscutils/bc.c
+++ b/miscutils/bc.c
@@ -844,10 +844,10 @@ struct globals {
 # error Strange INT_MAX
 #endif
 
-#if UINT_MAX == 4294967295
+#if UINT_MAX == 4294967295U
 # define BC_MAX_SCALE_STR  "4294967295"
 # define BC_MAX_STRING_STR "4294967294"
-#elif UINT_MAX == 18446744073709551615
+#elif UINT_MAX == 18446744073709551615U
 # define BC_MAX_SCALE_STR  "18446744073709551615"
 # define BC_MAX_STRING_STR "18446744073709551614"
 #else
-- 
2.18.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Patches to make GNU gzip and BusyBox gzip produce identical compression results

[Kang-Che Sung]

Well, it seems that Denys merged the changes before I have the time to
respond to comments, but anyway:

I actually disliked the argument about "de-facto standard" on compression
levels. I think scripts should not depend on the default compression level,
and should instead specify it explicitly. The reason is that you can not
guarantee the 'gzip' on one machine is the same implementation on another
(there are implementations that use more extensive search, for example,
7-zip and zopfli), it is just a coincidence that BusyBox gzip uses an
implementation that's compatible with the most popular (zlib, I think), but
it shouldn't be "guaranteed", let alone becoming a "standard" on
implementaion or on the default behaviors.

I am not suggesting to change the patch now. I'm just mentioning that
relying on default behavior of any program, for any script, is a bad idea
in general. (E.g. Do specify -6 if your script expects it, even though it
seems redundant, because user setting can always override the program
defaults.)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Patches to make GNU gzip and BusyBox gzip produce identical compression results

[Kang-Che Sung]

> From 12d30559486502feec4e2821b3ab45ae6139e7aa Mon Sep 17 00:00:00 2001
> From: Daniel Edgecumbe 
> Date: Mon, 2 Sep 2019 22:09:15 +0100
> Subject: [PATCH 3/3] gzip: set default compression level to 6 when
>  CONFIG_FEATURE_GZIP_LEVELS=n
>
> With this change, GNU gzip -n and BusyBox gzip now produce identical output
> assuming that CONFIG_GZIP_FAST=2.
> ---

Excuse me, but I wonder one thing on the third patch: Why should we follow
strictly with gzip on the no-options default behavior? gzip -9 is quite fast
in modern processors, and if someone builds busybox without
CONFIG_FEATURE_GZIP_LEVELS, I think they are moke likely to stick with -9 as
default instead of -6.

The better change would be to allow the builder to choose the compression level
at build time. It would be better to resolve the debate on which level should
be the default, Otherwise, I think the third patch can be dropped.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: switch_root: zap the last directory for the mount point of new-root

[Kang-Che Sung]

On Friday, August 2, 2019, 阿保 純一  wrote:
>> Speaking of, since we are now overmounting the root before zapping the
>> initramfs, I wonder if we can remove one check about whether the new root
>> is a mount point (this saves code size; mount() would fail with EINVAL in
>> that case).
> At least, I must check other mount points.
> For example, below is the init-script I'm using in initramfs.
>
> #!/bin/sh
> mount -t vfat /dev/sda2 /disk
> mount -o loop,ro -t squashfs /disk/squash.img /base
> mount -o loop -t xfs /disk/xfs.img /vary
> mount -o lowerdir=/base,upperdir=/vary/rootfs,workdir=/vary/work \
> -t overlay overlay /root
> mount --move /disk /root/initfs/disk
> mount --move /base /root/initfs/base
> mount --move /vary /root/initfs/vary
> my_standalone_switch_root -c /dev/tty1 /root /sbin/init
>
> I think switch_root in busybox is written already so dense, so every it's
check-codes treats not only the new-root but also other storages.
>

What I suggested is to remove these lines:

if (st.st_dev == rootdev) {
// Show usage, it says new root must be a mountpoint
bb_show_usage();
}

Because mount(..., MS_MOVE) will do the same check before moving the mount
point, and the check would be done before deleting anything if your patch
is applied. (And there is no point to show the program usage when it's
already PID 1. You would rather see error message of what specifically goes
wrong. The kernel will panic when PID 1 dies, leaving you no option but
reboot.)

The delete_contents() function in switch_root.c already prevents itself
from deleting files of other filesystems.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: switch_root: zap the last directory for the mount point of new-root

[Kang-Che Sung]

On Friday, July 19, 2019, Kang-Che Sung  wrote:
>
> There's side benefit for this patch: In case that overmount fails, we can
have
> a rootfs kept intact (instead of almost destroyed).
>

Correction. It's just a side effect, not a "benefit" worth talking about.

Speaking of, since we are now overmounting the root before zapping the
initramfs, I wonder if we can remove one check about whether the new root
is a mount point (this saves code size; mount() would fail with EINVAL in
that case).
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: switch_root: zap the last directory for the mount point of new-root

[Kang-Che Sung]

On Fri, Jul 19, 2019 at 3:27 PM 阿保 純一  wrote:
>
> As the author said in the comment of util-linux/switch_root.c, current 
> implementation leaves the mount point of new root-file-system without rmdir().
> As long as I experimented on a linux kernel, current process of "/" still 
> points old root-file-system even "/" is overmounted. So we can still access 
> and zap ititramfs after the directory is free from mount point.
>
> The patch below should zap the last directory left in the initramfs.
> It only swaps the timings of overmount and zapping.
>
> diff -Naur busybox-1.31.0.org/util-linux/switch_root.c 
> busybox-1.31.0/util-linux/switch_root.c
> --- busybox-1.31.0.org/util-linux/switch_root.c 2019-07-18 23:18:54.791346155 
> +0900
> +++ busybox-1.31.0/util-linux/switch_root.c 2019-07-18 23:21:33.867785730 
> +0900
> @@ -257,14 +257,14 @@
> }
>
> if (!dry_run) {
> -   // Zap everything out of rootdev
> -   delete_contents("/", rootdev);
> -
> // Overmount / with newdir and chroot into it
> if (mount(".", "/", NULL, MS_MOVE, NULL)) {
> // For example, fails when newroot is not a mountpoint
> bb_perror_msg_and_die("error moving root");
> }
> +
> +   // Zap everything out of rootdev
> +   delete_contents("/", rootdev);
> }
> xchroot(".");
> // The chdir is needed to recalculate "." and ".." links

There's side benefit for this patch: In case that overmount fails, we can have
a rootfs kept intact (instead of almost destroyed).

I think you should adjust the comment line:
// Zap everything out of (old) rootdev, where "/" still points to before chroot
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] wget: add -o flag

[Kang-Che Sung]

On Wednesday, December 26, 2018, Martin Lewis 
wrote:
> [...]
> @@ -147,6 +148,7 @@
>  //usage: "\n   -T SEC  Network read timeout is SEC
seconds"
>  //usage:   )
>  //usage: "\n   -O FILE Save to FILE ('-' for stdout)"
> +//usage: "\n   -o FILE Save output to FILE ('-' for
stdout)"

Am I the only one who think "output" is confusing for the "-o" option?

GNU wget manual already changed the name to "logfile" for what you are
implementing.

Speaking of, would you mind also change the "-O" help text to clarify it
refer to the "downloaded data"? Since both can be technically considered
wget "output" it helps to distinguish between the two.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Tweaks to build process for embedded scripts

[Kang-Che Sung]

On Monday, November 19, 2018, Ron Yorston  wrote:
> Kang-Che Sung wrote:
>>Script stripping should be optional, for at least two reasons:
>>1. It's beyond the scope of the script embedding feature, and it would
better
>>be implemented and maintained as a separate tool.
>
> I don't think it's out of scope.  If you're handing your scripts over
> to the tender mercies of the BusyBox build process you should expect
> stuff to happen to them.
>
> Letting authors indulge their literary aspirations without bloating the
> binary seems a nice feature to have.
>
>>2. Vendor may sign the scripts or publish their hashes or do something
with
>>them so that every bit of the script must remain intact.
>
> I suspect that most authors won't care.  Those who do need their scripts
> to be untouched should just ensure the first line doesn't match either
> of the regular expressions.  The old csh hack of putting ': /bin/sh'
> on the first line would do, for example.  And has a nice retro feel.
>

Dammit. Why should I workaround my script just for an ugly
"feature" you employ if I were building it?
This isn't funny. Just take it off. Or make it _optional_.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Tweaks to build process for embedded scripts

[Kang-Che Sung]

On Mon, Nov 19, 2018 at 5:43 PM Ron Yorston  wrote:
>
> Denys Vlasenko wrote:
> >On Sun, Nov 18, 2018 at 10:06 AM Ron Yorston  wrote:
> >> - Strip leading comments and blank lines from embedded scripts before
> >>   compressing them.  Removing all comments would be nice but is hard.
> >
> >Well, this last item can mangle scripts. Let's not do this.
>
> What's the concern?
>
> The awk script that removes the lines is:
>
> /^#/ { if (!found) next; }
> /^$/ { if (!found) next; }
> { found=1; print; }
>
> It's pretty conservative in what it deletes.  As soon as it finds a
> line that doesn't match its definition of a comment it copies the
> rest of the script verbatim.
>
> It would break scripts that use a shebang line to run a different
> interpreter but they aren't supported in this context anyway.

Script stripping should be optional, for at least two reasons:
1. It's beyond the scope of the script embedding feature, and it would better
be implemented and maintained as a separate tool.
2. Vendor may sign the scripts or publish their hashes or do something with
them so that every bit of the script must remain intact.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/3] Support both custom scripts and scripted applets

[Kang-Che Sung]

On Sat, Nov 17, 2018 at 6:56 AM Denys Vlasenko  wrote:
> I basically agree.
>
> How about this? - embed/ applets can have configuration.
> If it exists, then they are visible in "make menuconfig"
> and can be selected or deselected.
> But if config is _absent_, they are included unconditionally.
>
> This seems to cover both cases.

A compromise approach? That sounds good.

However, I'm still in the position that BusyBox shouldn't be a supplier of
scripted applets (as I said, that would complicate the configuration too much).
If you downstream are okay to deploy scripted applets, then go ahead. I just
don't wish dependency hell that's essentially bikeshedding (i.e. script applets
too trivial to implement) to come up here.

On Wed, Nov 14, 2018 at 7:54 PM Ron Yorston  wrote:
> My view is that 'scripted applets' should require configuration in the
> same way as native applets:
>
> - it should be possible to enable and disable them individually
> - they should be listed (in alphabetical order!) by 'busybox --help' and
>   'busybox --list'
> - they should be installed by 'busybox --install'
> - they should respond to 'busybox --help name' and 'name --help'
>
> The infrastructure for all of this is already present.
>
> Custom scripts on the other hand should require no configuration, apart
> from just dropping them in the 'embed' directory.  If 'embed' is empty
> the code to support custom scripts won't be present in the binary.
>
> I'd prefer to maintain this distinction and use the same 'embedded
> scripts' machinery to support deployment of both types of script.
>
How about this:

For the 'scripted applets' use case, you provide Kbuild and Config files just
like other applets, except they just happen to reside in embed directory.

For custom scripts, require at least a "list file" to indicate which should be
built and embedded into the binary, and also in what order they would present
in the 'busybox --help' output (they need not be in alphabetical order).

Scripts in the embed directory but without any _directive_ on how they should
be built would be ignored.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Add a 'busybox --list name' option to display script content Re: License concerns when embedding script in busybox binary

[Kang-Che Sung]

On Sat, Nov 10, 2018 at 2:10 AM Ron Yorston  wrote:
>
> Add an option to allow the content of embedded scripts to be
> displayed.
>
> It's disabled by default.  When enabled:
>
> function old new   delta
> run_applet_and_exit  728 801 +73
> .rodata   168561  168610 +49
> --
> (add/remove: 0/0 grow/shrink: 2/0 up/down: 122/0) Total: 122 bytes
>
Have you seen the implementation of bbconfig applet?
I wonder if the extraction of the embedded script can be
implemented in the same manner.
Will implementing that way yield smaller binary?

Sorry for such a bikeshedding argument.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

License concerns when embedding script in busybox binary

[Kang-Che Sung]

Since we are introducing the feature of embedding scripts into BusyBox, and
user may now include and build their custom scripts into the BusyBox binary, I
think there's one thing we forgot to address when this binary would be
distributed. That is: the license problem.

Specifically, I think the current state of config ASH_EMBEDDED_SCRIPTS help
text did not yet warn builders that the binary may be distributed **only when
the embedding scripts are GPLv2-compatible**. Builder and distributors may
accidentally violate copyright if they didn't get careful at reading the
BusyBox licenses. I think something should be done to make the warning more
explicit.

I'm not sure if additional measures needs to be done to check the script's
license (is it good to require every script in the embed directory to carry a
licence tag, or is that overkill?)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC PATCH v2] Allow applets to be implemented as scripts

[Kang-Che Sung]

On Wed, Nov 7, 2018 at 9:54 PM Tito wrote:
>
> this embedded scripts patch looks like "featuritis" at its best to me.
> It is adding complexity for solving what problem exactly:
> avoiding to copy the scripts manually to the new system or to
> the new firmware image?

I see one advantage of embedding scripts into busybox binary:
compression. The script could remain compressed in a firmware or
read-only image, saving space, and is useful for users who build
busybox only to run a specific script (installation, embedded
system startup, or something else non-trivial).
I think it's okay for that use case.

> The complexity added to the config system and to bb's common app init
> code seems not worth it.
I can't say about the common init code, but I do think adding
complexity to config system is bad, and that's why I'm against the
script-as-applet feature (especially with applet dependencies that could
be hell).

> One side effect I fear will derive from this patch is that
> users which would be capable of sending patches for bugs
> they find in bb will resort to simply scrap the applet and add
> an embedded script as it is faster.
I disagree. Embedded script always has an overhead of starting a shell
interpreter, although I acknowledge the size advantage the script can
provide, as long as many coreutils are present in that environment.
The script won't worth embedding if you don't intend to build a shell or
coreutils in busybox.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC PATCH 0/1] Allow applets to be implemented as scripts

[Kang-Che Sung]

On Wednesday, November 7, 2018, Ron Yorston  wrote:
> Kang-Che Sung wrote:
>>You have already been needing 4 lines of config options for dependencies
>>(CAT & SLEEP & ECHO & SH) of a 3-line script applet (nologin).
>
> nologin is actually an extreme case among the samples in applets_sh.
> The others only require sed, though each does have a TODO suggesting
> they could also use getopt.

If you mean applets_sh/dos2unix and unix2dos, than sorry, in the current
state of the scripts (master branch), they require `sed` and `test` (don't
overlook that square brackets). So you are wrong in this case.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC PATCH v2] Allow applets to be implemented as scripts

[Kang-Che Sung]

On Wed, Nov 7, 2018 at 4:54 PM Ron Yorston  wrote:
>
> Kang-Che Sung wrote:
> >I think there is a potential for user to modify script applets heavily (for
> >their particular application). Trying to track dependencies after user
> >modification would be too much work for little benefit.
>
> Sure, users can modify script applets and break them.  But that's nothing
> new:  even now they can modify C applets and break them.
>
> All that BusyBox can guarantee is that the applets it ships work as-is.
>
You have already been needing 4 lines of config options for dependencies
(CAT & SLEEP & ECHO & SH) of a 3-line script applet (nologin).
How can we guarantee that there would be no more
dependency hell if more script applets are brought in to busybox?
That's the problem.
And that's why I'm against the idea of configurable script applets.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC PATCH v2] Allow applets to be implemented as scripts

[Kang-Che Sung]

On Wed, Nov 7, 2018 at 3:42 PM Ron Yorston  wrote:
>
> Kang-Che Sung wrote:
> >Let no script applets individually configurable in menuconfig. Let users
> >resolve the dependencies of whatever they put in applets_sh.
>
> My view is that if script applets are provided as part of BusyBox they
> should have all the features of native applets, including the usual
> configuration facilities.
>
> The 'embed' directory is available for user-provided scripts.  These
> don't get the same level of support and it's up to the user to ensure
> their dependencies are met.
>
I have no opinion about embed directory in particular. My concern is about
potential config complexities if we ship script applets, especially non-trivial
ones.

All applets that you may implement as scripts have the potential to be
reimplemented in C. Rather than resolving applet dependencies for script
applets, making dependencies on only libc and libbb features could avoid all
dependency disasters we could have for introducing script applets.

Even seemingly trivial applet like nologin can bring in complexities. For
example, should I use echo or printf for printing messages? And what if choose
to _not_ support /etc/nologin.txt and use only hard-coded messages?

I think there is a potential for user to modify script applets heavily (for
their particular application). Trying to track dependencies after user
modification would be too much work for little benefit.

I would personally vote for that no script applets would be provided for
busybox, and that any script applet embedded in busybox would be user-provided
applet.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC PATCH v2] Allow applets to be implemented as scripts

[Kang-Che Sung]

Can we do this instead:

Let no script applets individually configurable in menuconfig. Let users
resolve the dependencies of whatever they put in applets_sh.

If user needs to temporarily not embed a particular applet in that folder,
then we can provide a "DISABLE" file that specifies which applets should
not embed to the binary.

In my opinion, if we were to resolve script applets' dependencies in
menuconfig, then the applets should have a potential to be re-implemented
with C, with libbb, which would reduce dependencies on other applets.

Don't complicate menuconfig with those dependencies they are not designed
to resolve.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/1] wget: make --no-check-certificate option appear in usage

[Kang-Che Sung]

On Sat, Jun 9, 2018 at 1:46 AM, James Byrne
 wrote:
> The line for this option was missing the 'usage:' prefix, meaning that
> it was ignored.
>
> Signed-off-by: James Byrne 
> ---

Well, actually wget in BusyBox does not yet check for certificates, so adding
the help text would be redundant and misleading.
What --no-check-certificate does for now is simply silencing a warning.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Kang-Che Sung]

On Fri, May 25, 2018 at 12:50 AM, Jakub Jirutka  wrote:
> Internal TLS code (FEATURE_WGET_HTTPS) does not implement validation
> of the server's certificate.  It is documented in the code, but not
> even mentioned in the --help message, so users typically don't know
> about this behaviour.  That's a crime against security!
>
> This patch changes this behaviour for the case when both
> FEATURE_WGET_LONG_OPTIONS and FEATURE_WGET_HTTPS are enabled - any
> attempt to open a TLS connection using internal TLS code (i.e. without
> certificate validation) ends with error, unless the user specified
> option "--no-check-certificate".
>

Jakub,

I wonder if you can revise your patch, so that it prints a warning that
certificate check is skipped instead of error-ing and quitting.
That way the patch might have a chance of getting accepted.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] wget: don't silently ignore certificate validation

[Kang-Che Sung]

On Sun, May 27, 2018 at 1:34 AM, Denys Vlasenko
 wrote:
> wget should work for common use cases.
> Such as downloading sources of kernels, gcc and such.
> From build scripts, not only by hand.
> Without having to modify said scripts.
> Your patch breaks that.
> NAK.
>
> I don't care that security people are upset.
> They are paranoid, it's part of their profession.
> It does not mean everybody else have to be as paranoid.
>
> If you have a patch which adds actual cert checking
> and thus does not introduce regressions, please post it.
>

I think I need to point out that in usability perspective, BusyBox's current
behaviour is not ideal. It should give a runtime warning that certificate
checks are skipped, instead of pass it silently. Of course, it would be better
if actual certificate check is implemented, but if builder disables it (for
binary size or simplicity), there should be a runtime warning so that usability
for secure people won't be compromised.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/2] udhcpc6: carry along length of packet when parsing it.

[Kang-Che Sung]

David Decotigny  於 2018年5月24日 週四 23:38 寫道：

> This is to avoid parsing garbage past packet's actual end.
>
> Also const-ize params to a few functions.
>
> Signed-off-by: David Decotigny 
>

I wonder if parsing such "garbage" has security implications.

>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [BUG] 'case' does not match double-quoted backslash

[Kang-Che Sung]

On Tue, Feb 13, 2018 at 10:34 AM, Martijn Dekker <mart...@inlv.org> wrote:
> Op 13-02-18 om 03:09 schreef Kang-Che Sung:
>> Just wondering, why using "\z" and not "\\z" ?
>> The former doesn't seem to be a valid syntax.
>
> It is valid, though. See the POSIX specification:
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_03
>
> ===begin quote===
> 2.2.3 Double-Quotes
> [...]
>
> \
> The  shall retain its special meaning as an escape
> character (see Escape Character (Backslash)) only when followed by one
> of the following characters when considered special:
>
> $   `   "   \   
> ===end quote===
>
> So, this should simply work. It worked on ash before. It also works on
> every other POSIX shell, and existing scripts rely on it.

It says the backslash is special only when followed by the $ ` " \
 characters.
That is, \$ \` \" \\ and \ are special, but none of these
includes the \z you
mentioned, so what you described is an undefined behavior.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [BUG] 'case' does not match double-quoted backslash

[Kang-Che Sung]

On Tue, Feb 13, 2018 at 5:36 AM, Martijn Dekker  wrote:
> The following outputs BUG in ash:
>
> case "\z" in
> "\z" )  echo ok ;;
> * ) echo BUG ;;
> esac
>
> Apparently `case` has trouble matching the "\z" pattern due to the
> backslash within the double quotes. Quoting it in any other way works.
>
> Also, backslash-escaping the backslash within the double quotes is a
> workaround. But this is not supposed to be necessary unless the
> backslash precedes one of the special characters $, `, ", \, or .
>
> The bug is in 1.28.0 but not 1.27.0.
>
> According to my testing, this bug appears to have been introduced by
> this commit, which fixed another related bug.
>
> | commit fda9fafe279d9394ad53313320a949c86f646734
>
> | Author: Denys Vlasenko 
> | Date:   Wed Jul 5 19:10:21 2017 +0200
>
> |
> | ash: fix matching of unicode greek letter rho (cf 81) and similar
> cases
> |
>
> | Signed-off-by: Denys Vlasenko 

Just wondering, why using "\z" and not "\\z" ?
The former doesn't seem to be a valid syntax.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: make bash_source command search current directory after PATH

[Kang-Che Sung]

I really want to argue that there is a quite simple workaround to get the
bash 'source' behavior you need: Just append ":." to the PATH environment
variable before you run the "." command.

Or this:
source () {
PATH="$PATH:." . "$@"
}

Was there any difficulty of doing it?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: make bash_source command search current directory after PATH

[Kang-Che Sung]

On Mon, Jan 29, 2018 at 2:51 AM, Bernd Petrovitsch
 wrote:
> On Fri, 2018-01-26 at 15:15 +0100, Denys Vlasenko wrote:
>> On Fri, Jan 26, 2018 at 9:34 AM, Paul Otto  wrote:
> [...]
>> > According to the BASH documentation, the source command should:
>> > Read and execute commands from filename  in  the  current  shell 
>> > environment
>> > and return the exit status of the last command executed from filename.  If
>> > filename does not contain a slash, filenames  in  PATH  are used to find 
>> > the
>> > directory containing filename.  The file searched for in PATH  need  not  
>> > be
>> > executable. When  bash  is  not  in  posix  mode,  the  current directory 
>> > is
>> > searched if no file is found in PATH.
>>
>> I wish bash wouldn't introduce gratuitous standard violations.
>
> I see such begaviour more as a security problem - there are good
> reasons not using "." automagically in $PATH (like DOS did .).
>

The problem with DOS/Windows is that they prioritize "." (working directory)
over PATH when searching commands, which can cause security problems.
Bash actually considers "." last, which is less of the security threat (unless
you're setting a shell environment when user doesn't even have permission to
adjust $PATH).

For reference, here is the rationale in POSIX:

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#dot

"Some older implementations searched the current directory for the [file],
even if the value of [PATH] disallowed it. This behavior was omitted from this
volume of POSIX.1-2008 due to concerns about introducing the susceptibility to
trojan horses that the user might be trying to avoid by leaving _dot_ out of
[PATH]."
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: make bash_source command search current directory after PATH

[Kang-Che Sung]

2018年1月28日 上午5:05，"Paul Otto" 寫道：

Thanks for taking care of this, Denys. It didn't wind up the way I'd hoped,
but at least it is predictable both ways now. I will see if there is a way
to get Alpine Linux to build with that config option set, to resolve the
regression introduced by busybox 1.27.


I wonder when people suggest adding a config option to make "source"/"."
command bash-compatible, why aren't they bothered to append the "."
directory at the end of $PATH and save the bloat on the shell code.

Also, I suggest the new BASH_SOURCE_CURDIR be _one_ config option across
both shells, instead of allowing different behaviors of two shells in one
binary. And since this is about behavior change and not a feature
"extension", I think we have no need for this config option to depend on
*_BASH_COMPAT.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 0/2] Add -R to less applet (bug #5546)

[Kang-Che Sung]

Hi Denys Vlasenko,

I think I found a typo in the recent commit
8528d3d4f8240ab4715f671aa819fe034f0fc285

> @@ -106,7 +122,7 @@
>
>  //usage:#define less_trivial_usage
>  //usage: "[-E" IF_FEATURE_LESS_REGEXP("I")IF_FEATURE_LESS_FLAGS("Mm")
> -//usage: "N" IF_FEATURE_LESS_TRUNCATE("S") "h~] [FILE]..."
> +//usage: "N" IF_FEATURE_LESS_TRUNCATE("S") IF_FEATURE_LESS_TRUNCATE("R") 
> "h~] [FILE]..."
>  //usage:#define less_full_usage "\n\n"
>  //usage: "View FILE (or stdin) one screenful at a time\n"
>  //usage: "\n -E Quit once the end of a file is reached"


It shouldn't be IF_FEATURE_LESS_TRUNCATE("R") .
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 4/4] chrt: don't rely on exact values of SCHED_* defines

[Kang-Che Sung]

On Sun, Jan 14, 2018 at 5:30 AM, Povilas Kanapickas  wrote:
> --- a/util-linux/chrt.c
> +++ b/util-linux/chrt.c
> @@ -36,17 +36,20 @@
>  #include 
>  #include "libbb.h"
>
> -static const struct {
> -   int policy;
> -   char name[sizeof("SCHED_OTHER")];
> -} policies[] = {
> -   {SCHED_OTHER, "SCHED_OTHER"},
> -   {SCHED_FIFO, "SCHED_FIFO"},
> -   {SCHED_RR, "SCHED_RR"},
> -   {SCHED_BATCH, "SCHED_BATCH"},
> -   {0 /* unused */, ""},
> -   {SCHED_IDLE, "SCHED_IDLE"}
> -};
> +static const char* get_policy_name(int pol)
> +{
> +   if (pol == SCHED_OTHER)
> +   return "SCHED_OTHER";
> +   if (pol == SCHED_FIFO)
> +   return "SCHED_FIFO";
> +   if (pol == SCHED_RR)
> +   return "SCHED_RR";
> +   if (pol == SCHED_BATCH)
> +   return "SCHED_BATCH";
> +   if (pol == SCHED_IDLE)
> +   return "SCHED_IDLE";
> +   return "";
> +}
>
>  static void show_min_max(int pol)
>  {

I don't like the chain of "if"s here. Have you checked the size changes on the
compiled code?
Since this SCHED policy constants are just enum values, what about using
"switch-case" statement? The problem of if-chains is that some compilers are
not smart at figuring out that the comparisons are just enums and can be
optimized. The switch-case statement will make it more clear.

Sorry for nit-picking the code.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ash.c history loading and ENABLE_FEATURE_EDITING_SAVEHISTORY

[Kang-Che Sung]

2017年12月24日 09:17，"Ceriel Jacobs" 寫道：

There is no need to remember what was just typed.
There is a need to use commands that were typed a long time (years) ago.


Disasters often happen a long time after the backup (script/system) was
made.


The point is that you don't have to:
1. remember the commands
2. type the commands
3. correct typing mistakes


I know that I can save .ash_history
but it seems I can't load it without feature ENABLE_FEATURE_EDITING_SAVEHISTORY
set.


And convincing Arch Linux developers to turn on the
ENABLE_FEATURE_EDITING_SAVEHISTORY feature will be difficult. Because in
their initramfs environment they don't want to have a SAVEHISTORY to file
functionality.


I'm not convinced by this. Since your use case is a very rare one
(read-only "history" somehow defeats the purpose of the history - and it
overlaps what pre-written shell function is for). While I'm not one who can
make decisions on this, I'd suggest you just keep your local patch to do
this.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ash.c history loading and ENABLE_FEATURE_EDITING_SAVEHISTORY

[Kang-Che Sung]

2017-12-24 07:55，"Ceriel Jacobs" 寫道：

It seems that BusyBox v1.27.2 doesn't load history from a history file when
ENABLE_FEATURE_EDITING_SAVEHISTORY is not set.

I think that the corresponding source code logic is here:


Such logic doesn't match the feature name, which is ...SAVEHISTORY and not
...LOADHISTORY.

For example:
Busybox is often used in (emergency/disaster) recovery environments. There
is no need for any saving (readonly media). However it comes handy when
frequently used commands can be pre-loaded into the initramfs (for
interactive logins).

Is such a loadhistory-without-savehistory scenario tackled when changing:

#if MAX_HISTORY > 0 && ENABLE_FEATURE_EDITING_SAVEHISTORY

to

#if MAX_HISTORY > 0

in ash.c?

In case it is not, what more to change to allow loading a HISTFILE when
ENABLE_FEATURE_EDITING_SAVEHISTORY is not set?


What's the point of loading frequently used commands if you don't save what
you have typed *just now*? You can have .ash_history in a temp filesystem
if you like, but it still counts as saving the history.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: fix a crash on arm when building with clang.

[Kang-Che Sung]

On Thu, Dec 21, 2017 at 5:32 AM, Yunlian Jiang  wrote:
>
> When I run
> busybox ash on an arm device with busybox built with clang 6.
> I got a segmentation fault
> in the macro
> #define INIT_S() do { \
> (*(struct lineedit_statics**)_ptr_to_statics) = xzalloc(sizeof(S)); \
> barrier(); \
> cmdedit_termw = 80; \
> IF_USERNAME_OR_HOMEDIR(home_pwd_buf = (char*)null_str;) \
> IF_FEATURE_EDITING_VI(delptr = delbuf;) \
> } while (0)
>
> With the patch below, the segmentation disappears.
>
> --- busybox-1.27.2/libbb/lineedit.c
> +++ busybox-1.27.2/libbb/lineedit.c
> @@ -197,8 +197,8 @@ extern struct lineedit_statics *const li
>   (*(struct lineedit_statics**)_ptr_to_statics) = 
> xzalloc(sizeof(S)); \
>   barrier(); \
>   cmdedit_termw = 80; \
> - IF_USERNAME_OR_HOMEDIR(home_pwd_buf = (char*)null_str;) \
> - IF_FEATURE_EDITING_VI(delptr = delbuf;) \
> + IF_USERNAME_OR_HOMEDIR(home_pwd_buf = (char*)null_str;); \
> + IF_FEATURE_EDITING_VI(delptr = delbuf;); \
>  } while (0)
>
>  static void deinit_S(void)

What's the problem of the missing semicolons in the build? BusyBox has been
using these constructs almost everywhere. And they should succeed.

Could you show us the generated code differences?
It looks like a bug in the compiler, and nothing to do with Busybox.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Ash glob symlink regression when using libc glob

[Kang-Che Sung]

On Tue, Oct 31, 2017 at 8:48 PM, Lauri Kasanen  wrote:
>
> So it was fixed after 2.26 and no released glibc contains the fix. How
> about either editing the description to say glibc >= 2.27 only, or even
> checking that compile-time?

I would like to have both. And probably give a preprocessor warning about
this, something like "warning: glibc <2.27 has buggy glob(); enabling
ash internal one"
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 4/7] xfuncs: Handle missing non-POSIX termios constants

[Kang-Che Sung]

On Sun, Oct 8, 2017 at 10:34 PM, Ralf Friedl  wrote:
>
> Because the way the line is now is one line with 8 values. Changing that to
> use ifdefs for each value would change that one line to 3*8+2=26 lines, that
> makes the code much harder to read and maintain. And there may be other
> places where it is used. It's perfectly legitimate to define the values as
> 0, like you would define O_BINARY as 0 on platforms that don't need/supply
> it.

Okay. I'm partially convinced about this argument. Consider that there are
other places in busybox code that temporarily defines IUCLC to 0 for similar
purpose:

libbb/xfuncs.c
libbb/bb_askpass.c
coreutils/stty.c
loginutils/getty.c

> There is nothing wrong with that. It is possible to use "#if SIGUSR3" to
> test for SIGUSR being defined and nonzero. It has the additional benefit
> that you can use the value in a C statement (as opposed to a C preprocessor
> test) and let the compiler optimize it out. Busybox does that a lot.

My concern is that there're some macros that are defined to 0 for a
purpose, e.g.
#define O_RDONLY 0x
so it's not a good idea in general to arbitrarily define a macro to 0 without
knowing what you are doing. But for IUCLC and such, it might be safe.

James (jrtc27), I apologize for my misunderstanding of the issue.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 7/7] libbb.h: Handle missing HOST_NAME_MAX; ensure MAXFOOLEN agrees with FOO_MAX

[Kang-Che Sung]

2017年10月8日 18:59，"James Clarke" 寫道：


That's not actually true (any more); util-linux/fdisk_osf.c, whilst
Linux-specific, does use MAXPATHLEN, and networking/traceroute.c uses
MAXHOSTNAMELEN. These could be changed to use {PATH,HOST_NAME}_MAX, but it's
highly likely that a future change will re-introduce a use of one of those
macros and break the build on the Hurd, so why not just define them?


Let's assume that Linux will provide these non-standard macros. You won't
build them in Hurd, so why bother?

The problem is that you can't assume the meanings and usage of two macros
are both the same. You might potentially break more for your "convenience"
definition that's technically unnecessary.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 4/7] xfuncs: Handle missing non-POSIX termios constants

[Kang-Che Sung]

2017年10月8日 18:50，"James Clarke" <jrt...@jrtc27.com>寫道：

On 8 Oct 2017, at 02:34, Kang-Che Sung <explore...@gmail.com> wrote:
> On Sun, Oct 8, 2017 at 1:53 AM, James Clarke <jrt...@jrtc27.com> wrote:
>> diff --git a/libbb/xfuncs.c b/libbb/xfuncs.c
>> index 9cbfb2836..95dac656a 100644
>> --- a/libbb/xfuncs.c
>> +++ b/libbb/xfuncs.c
>> @@ -22,6 +22,16 @@
>>  */
>> #include "libbb.h"
>>
>> +#ifndef IMAXBEL
>> +# define IMAXBEL 0
>> +#endif
>> +#ifndef IUCLC
>> +# define IUCLC 0
>> +#endif
>> +#ifndef IXANY
>> +# define IXANY 0
>> +#endif
>> +
>
> I wonder, how do these break, and why does defining them as 0 solve the
problem?

FreeBSD (well, GNU/kFreeBSD at least) does not define IUCLC. They are only
used
in the line:

> newterm.c_iflag &= ~(BRKINT|INLCR|ICRNL|IXON|IXOFF|IUCLC|IXANY|IMAXBEL);

Thus, by defining them as 0 when not present, it is as if they were omitted
from that list, without needing a bunch of confusing #ifdef's in the
expression
itself. While IMAXBEL and IXANY are defined everywhere Debian cares about,
they
are still non-standard, so I did the same for them in case there is a
platform
out there without them.

Regards,
James


Why not just fix that usage line so that the bitwise OR's are all enclosed
in #ifdef lines? I think it's better than defining the macros to a value
that may cause further problems when reused.

Analogy: You probably won't define an unsupported signal, say SIGUSR3, to a
zero value anyway. They are undefined for a reason. Hope you understand.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 7/7] libbb.h: Handle missing HOST_NAME_MAX; ensure MAXFOOLEN agrees with FOO_MAX

[Kang-Che Sung]

On Sun, Oct 8, 2017 at 1:53 AM, James Clarke  wrote:
> Signed-off-by: James Clarke 
> ---
>  include/libbb.h | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/libbb.h b/include/libbb.h
> index daccf154a..56f4f4cb3 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -181,7 +181,24 @@ extern char **environ;
>  /* klogctl is in libc's klog.h, but we cheat and not #include that */
>  int klogctl(int type, char *b, int len);
>  #ifndef PATH_MAX
> -# define PATH_MAX 256
> +# ifdef MAXPATHLEN
> +#  define PATH_MAX MAXPATHLEN
> +# else
> +#  define PATH_MAX 256
> +# endif
> +#endif
> +#ifndef MAXPATHLEN
> +# define MAXPATHLEN PATH_MAX
> +#endif
> +#ifndef HOST_NAME_MAX
> +# ifdef MAXHOSTNAMELEN
> +#  define HOST_NAME_MAX MAXHOSTNAMELEN
> +# else
> +#  define HOST_NAME_MAX 256
> +# endif
> +#endif
> +#ifndef MAXHOSTNAMELEN
> +# define MAXHOSTNAMELEN HOST_NAME_MAX
>  #endif
>  #ifndef BUFSIZ
>  # define BUFSIZ 4096


Um, why do we need to define MAXPATHLEN and MAXHOSTNAMELEN where there have
been no use in BusyBox, and there have been standard macros available?

While we are at it, could you also please change the HOST_NAME_MAX fallback
definition to 255? 255 is the POSIX defined minimum.

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html
(see _POSIX_HOST_NAME_MAX value)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 4/7] xfuncs: Handle missing non-POSIX termios constants

[Kang-Che Sung]

On Sun, Oct 8, 2017 at 1:53 AM, James Clarke  wrote:
> diff --git a/libbb/xfuncs.c b/libbb/xfuncs.c
> index 9cbfb2836..95dac656a 100644
> --- a/libbb/xfuncs.c
> +++ b/libbb/xfuncs.c
> @@ -22,6 +22,16 @@
>   */
>  #include "libbb.h"
>
> +#ifndef IMAXBEL
> +# define IMAXBEL 0
> +#endif
> +#ifndef IUCLC
> +# define IUCLC 0
> +#endif
> +#ifndef IXANY
> +# define IXANY 0
> +#endif
> +


I wonder, how do these break, and why does defining them as 0 solve the problem?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Cannot download busybox with busybox

[Kang-Che Sung]

On Tue, Aug 29, 2017 at 7:58 PM, Denys Vlasenko
 wrote:
> I'll cc this to bbox mailing list to humiliate this guy.
>
> On Tue, Aug 29, 2017 at 1:21 PM,   wrote:
I'm trying to update busybox binary on busybox-based system.
When i am trying to download busybox, i have this error

# wget
http://busybox.net/downloads/binaries/1.26.1-defconfig-multiarch/busybox-mips
Connecting to busybox.net[140.211.167.122]:80
wget: not an http or ftp url:
https://busybox.net/downloads/binaries/1.26.1-defconfig-multiarch/busybox-mips

Why you are redirecting to https when https is not supported in most
builds (And my system is too weak to use openssl)? It is some kind of
trolling?
>>>
>>> Support for https is added in last released version
>>
>> OK, but what if you ask for p7zip and get p7zip.7z file? What you use to
>> unpack it? And how i can download new version with https support without
>> https support from https-only server? It is stupid like windows device
>> driver wizard trying to find network device drivers in internet.
>
> I don't remember you having a contract with me to support your operations.
>
> On what moral or legal grounds are you complaining that I did not
> write TLS support for wget fast enough for Your Majesty's taste?

So he asked for this feature by mailing the maintainer personally?
And why was he demanding a prebuilt binary in the first place? (You know, a
default configuration for busybox rarely fits your client / embedded system's
requirement.)

And the analogy of p7zip is a failure. Every FOSS package has its own
bootstrapping problem to address. GCC needs a C++ compiler; tar needs a copy of
tar to unpack its own sources. The upstream maintainers may give you
convenience by providing source packages in other formats (e.g. shar),
but demanding someone to ship it in the way you want is rude (we don't owe you
anything).

So what's preventing him from downloading busybox from another computer,
build it, and copy it to his embedded MIPS system through LAN or a USB stick?

Enough said.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] ps: Allow ps config options if minips is enabled

[Kang-Che Sung]

Follow-up of commit ab77e81a8527fa11a4f9392d97c2da037d6f4f98
"klibc-utils: new applets: resume, nuke, minips"

Also put FEATURE_PS_UNUSUAL_SYSTEMS to under FEATURE_PS_TIME in the
menu.

Signed-off-by: Kang-Che Sung <explore...@gmail.com>
---
 procps/ps.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/procps/ps.c b/procps/ps.c
index 7edf6dbd1..fe2d8c294 100644
--- a/procps/ps.c
+++ b/procps/ps.c
@@ -17,7 +17,7 @@
 //config:config FEATURE_PS_WIDE
 //config:  bool "Enable wide output (-w)"
 //config:  default y
-//config:  depends on PS && !DESKTOP
+//config:  depends on (PS || MINIPS) && !DESKTOP
 //config:  help
 //config:  Support argument 'w' for wide output.
 //config:  If given once, 132 chars are printed, and if given more
@@ -26,7 +26,7 @@
 //config:config FEATURE_PS_LONG
 //config:  bool "Enable long output (-l)"
 //config:  default y
-//config:  depends on PS && !DESKTOP
+//config:  depends on (PS || MINIPS) && !DESKTOP
 //config:  help
 //config:  Support argument 'l' for long output.
 //config:  Adds fields PPID, RSS, START, TIME & TTY
@@ -34,14 +34,9 @@
 //config:config FEATURE_PS_TIME
 //config:  bool "Enable -o time and -o etime specifiers"
 //config:  default y
-//config:  depends on PS && DESKTOP
+//config:  depends on (PS || MINIPS) && DESKTOP
 //config:  select PLATFORM_LINUX
 //config:
-//config:config FEATURE_PS_ADDITIONAL_COLUMNS
-//config:  bool "Enable -o rgroup, -o ruser, -o nice specifiers"
-//config:  default y
-//config:  depends on PS && DESKTOP
-//config:
 //config:config FEATURE_PS_UNUSUAL_SYSTEMS
 //config:  bool "Support Linux prior to 2.4.0 and non-ELF systems"
 //config:  default n
@@ -49,6 +44,11 @@
 //config:  help
 //config:  Include support for measuring HZ on old kernels and non-ELF 
systems
 //config:  (if you are on Linux 2.4.0+ and use ELF, you don't need this)
+//config:
+//config:config FEATURE_PS_ADDITIONAL_COLUMNS
+//config:  bool "Enable -o rgroup, -o ruser, -o nice specifiers"
+//config:  default y
+//config:  depends on (PS || MINIPS) && DESKTOP
 
 // APPLET_NOEXEC:namemain locationsuid_type help
 //applet:IF_PS(APPLET_NOEXEC(ps, ps,  BB_DIR_BIN, BB_SUID_DROP, ps))
-- 
2.14.1

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash/hush: implement -d DELIM option for `read`

[Kang-Che Sung]

On Mon, Aug 7, 2017 at 6:18 PM, Johannes Schindelin
 wrote:
> The POSIX standard only requires the `read` builtin to handle `-r`:
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html
>
> However, Bash introduced the option `-d ` to override IFS for
> just one invocation, and it is quite useful.
>
> It is also super easy to implement in BusyBox' ash, so let's do that.
>
> The motivation: This option is used by Git's test suite.
>
> Signed-off-by: Johannes Schindelin 

Can you wrap the change within a macro conditional like #if BASH_READ_D ?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: BusyBox top buffered and the cached memory

[Kang-Che Sung]

On Tue, Aug 1, 2017 at 3:44 PM, Klaaßen, Fynn (LAWO)
 wrote:
>
> I guess, there is a trivial bug when using busybox top:
>
> BusyBox v1.22.1 (Ubuntu 1:1.22.0-15ubuntu1) multi-call binary:
>
> Mem: 3278296K used, 1236784K free, 0K shrd, 35373432K buff, 140001458828604K
> cached
>
> …or…
>
> BusyBox v1.22.1:
>
> Mem: 559252K used, 1413876K free, 0K shrd, 160503248K buff, 160503296K
> cached
>
> The size oft the buffered and the cached memory seems a bit large and
> probably has the wrong unit.

Can you reproduce it? Did you try the latest development version (git
master branch)?
I think you can help us more if you can deduce which commit caused the
regression,
or provide some other info that can help us reproduce it.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] libbb: remove vdprintf

[Kang-Che Sung]

On Sat, Jul 29, 2017 at 9:48 PM, Ron Yorston  wrote:

> -#if defined(__GLIBC__) && __GLIBC__ < 2
> -int vdprintf(int d, const char *format, va_list ap);
> -#endif

__GLIBC__ is NOT defined in glibc before version 2.
So this macro check actually means a libc which claims to be
compatible with glibc but not glibc 2.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: support platforms that don't have '%m' printf specifier

[Kang-Che Sung]

On Fri, Jul 28, 2017 at 12:58 AM, Jody Bruchon  wrote:

> IMO the whole issue is that %m is non-standard despite having moderate de
> facto acceptance in many libc implementations. I think BusyBox should comply
> with the standards instead of having a compile-time hack and added code
> complication to save a couple of bytes by switching to %m. There is no
> guarantee that a particular BusyBox binary built to use a library with %m
> won't be moved from that machine to one with the same libc but without %m
> built in (I don't know if dietlibc could bring such a situation to reality
> but it wouldn't surprise me.)

That's a good question, and a good reason to leave %m disabled in
default busybox build for dietlibc.

But better leave a comment line in platform.h there to state the reason.

> As far as I can see the only savings by using
> %m would be one parameter off of a [f]printf() and one function call to
> strerror() removed; how much is that really saving, and is it worth the
> added code complexity and the non-compliance with the standards?

The decision isn't by me, but it seems that BusyBox concerns more about
binary size than standard compliance. So we may use extensions when
they reduce the binary size, even by only a few bytes.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: support platforms that don't have '%m' printf specifier

[Kang-Che Sung]

On Thu, Jul 27, 2017 at 8:53 PM, Ron Yorston  wrote:
> The '%m' conversion specifier prints an error message based on the
> current value of 'errno'.  It is available in the GNU C library,
> Cygwin (since 2012), uClibc and musl.
>
> It is not available in various BSDs, BSD-derived systems (MacOS,
> Android) or Microsoft Windows.
>
> Use a symbol defined in platform.h to control how error messages
> can be formatted to display the 'errno' message.  On platforms that
> support it use '%m'; on other platforms use '%s' and strerror().
>

dietlibc supports %m only if WANT_ERROR_PRINTF is defined in
 when building, and its commented out by default.
I don't know how to check the macro WANT_ERROR_PRINTF in
the dietlibc build environment, but I think busybox needs to be
aware of this.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: avoid GLIBC'ism %m

[Kang-Che Sung]

On Mon, Jul 24, 2017 at 3:34 AM, Denys Vlasenko
<vda.li...@googlemail.com> wrote:
> On Sat, Jul 22, 2017 at 8:56 PM, Johannes Schindelin
> <johannes.schinde...@gmx.de> wrote:
>> On Fri, 21 Jul 2017, Denys Vlasenko wrote:
>>
>>> On Wed, Jul 19, 2017 at 3:47 AM, Jody Bruchon <j...@jodybruchon.com> wrote:
>>> > On 2017-07-18 9:15 PM, Kang-Che Sung wrote:
>>> >>
>>> >> On Wed, Jul 19, 2017 at 2:11 AM, Markus Gothe <nietzs...@lysator.liu.se>
>>> >> wrote:
>>> >>>
>>> >>> Actually last time I checked ‘%m’ is POSIX contrary to glibc’s 
>>> >>> deprecated
>>> >>> '%a’. However, I agree that it should not be used since at least uClibc 
>>> >>> can
>>> >>> be built without support for it.
>>> >>
>>> >> How come %m is POSIX when I didn't see any mention of it in this page?
>>> >> http://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html
>>> >
>>> > It does not appear to be part of POSIX or the Single UNIX Specification. 
>>> > The
>>> > glibc man page (as of 2016-12-12) even indicates that it is a 
>>> > glibc-specific
>>> > extension:
>>> >
>>> > *m *(Glibc extension; supported by uClibc and musl.) Print output of
>>> > /strerror(errno)/. No argument is required.
>>>
>>> This sounds like every libc has already conceded to implementing it.
>>>
>>> Let's benefit from it then?
>>
>> No, not every libc. I would not have spent the time and effort to develop
>> the patch, contribute it, rework it and contribute a second iteration if
>> it was not for a good reason now, would I.
>
> Good point.
> What libc is that?

Perhaps FreeBSD counts here.
https://www.freebsd.org/cgi/man.cgi?printf(3)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH v3 0/2] Avoid GLIBC'ism "%m"

[Kang-Che Sung]

On Tue, Jul 25, 2017 at 9:56 PM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:
>
> As pointed out by Kang-Che Sung and verified by Jody Bruchon, the "%m"
> format placeholder is really not in any open standard.
>
> Also: contrary to Denys' assumption, there are libc versions out there
> which did not follow GLIBC's example to implement this non-POSIX
> placeholder. That must be the reason why nothing else in BusyBox' source
> code relies on %m but uses helpers such as bb_perror_msg() instead.
>
> It is very easy, and a good readability improvement, too, to introduce
> a new helper in ash's source code.

My personal arguments are:
1. If we want to use printf %m, then make sure we support that wherever
possible. Even bb_perror_msg and like could be extended to use %m.
2. It's good to provide fallback for libc that doesn't support %m
anyway. That's what platform.c & platform.h in busybox are all about.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: avoid GLIBC'ism %m

[Kang-Che Sung]

On Sun, Jul 23, 2017 at 10:03 AM, Kang-Che Sung <explore...@gmail.com> wrote:
>
> How about wrapping the printf("%m") uses within the __GNU_LIBRARY__ macro?
> It seems that %m support has been there from the beginning of glibc.

Correction. It's since glibc 1.06.

I've looked in the old ChangeLog entries and saw this:

Tue Oct 20 18:36:40 1992  Roland McGrath  (rol...@geech.gnu.ai.mit.edu)
* configure: Write error message and lose for option with missing arg.
* stdio/__vfscanf.c: Add `a' modifier, which makes %s and %[ fill
in a char ** with a malloc'd string.
(STRING_ADD_CHAR, STRING_ARG): New macros to deal with this hair.
(%s, %[): Use them.
* posix/gnu/types.h [__GNUC__] (__fsid_t): Define as long long.
* stdio/vfprintf.c: Add %m, which is %s of strerror (errno).

So... the check would be probably like this:
#if __GNU_LIBRARY__ > 1
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: avoid GLIBC'ism %m

[Kang-Che Sung]

On Sun, Jul 23, 2017 at 4:33 AM, Michael Conrad  wrote:
> On 7/22/2017 2:56 PM, Johannes Schindelin wrote:
>>
>> No, not every libc. I would not have spent the time and effort to develop
>> the patch, contribute it, rework it and contribute a second iteration if
>> it was not for a good reason now, would I.
>>
>
> I believe his point is that your patch adds size to busybox which is
> unneeded for most users.  (btw, it's recommended to post bloatcheck numbers
> with a patch.  If you show a small number from bloatcheck then there is less
> to argue about)
>
> Which libc are you using?  Do you think %m support could be checked with an
> ifdef?
>

How about wrapping the printf("%m") uses within the __GNU_LIBRARY__ macro?
It seems that %m support has been there from the beginning of glibc.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: About the approximate applet sizes in menuconfig

[Kang-Che Sung]

On Thu, Jul 20, 2017 at 8:22 PM, walter harms <wha...@bfs.de> wrote:
>
>
> Am 20.07.2017 12:17, schrieb Denys Vlasenko:
>> On Wed, Jul 19, 2017 at 6:38 PM, Kang-Che Sung <explore...@gmail.com> wrote:
>>> (https://git.busybox.net/busybox/commit/?id=4eed2c6c5092ed95b8ee6d994106c54a9fc6ed3e)
>>>
>>> I don't like this. I don't like these size info to be put on the titles
>>> of config options. A few reasons:
>>> * It can give a false impression that the size of the final busybox
>>>   binary is the size of these applets summed up, but it's actually not.
>>>   (Some code can share among applets)
>>> * The sizes are CPU architecture dependent.
>>
>> Sure, on a different arch, sizes would change, but "20k" applet is
>> still approximately 4 times larger than "5k" one.
>> that's useful information for the user.
>>
>
> to be korrekt we would need to test with every new compiler/-option more over
> nobody knows when this claim was made. It should be clear for every programmer
> "your milage may vary" is here in big letters.

I can assume the size is measured with the default compiler options with a
modern compiler -- nowadays building with GCC and building with Clang do not
bring so much size differences, but it's still good if we leave a note about
which compiler is used on measuring size.

And I used the word "about" for the "your mileage may vary" meaning.

>>> * How the sizes are measured are even unclear. So, does the size include
>>>   all optional features of the applet, or is it only the basic
>>>   functionality?
>>>
>>> If I were to mention the size in menuconfig, I would put in the help
>>> texts of the config options, rather than their titles. And be clear how
>>> the size is measured, for example:
>>>
>>>CONFIG_BINZIP2
>>>(Adds about 8.8kB on i686, for bzip2 decompression code)
>>>CONFIG_BZIP2
>>>(Adds about 18kB on i686, for bzip2 compression and decompression code)
>>
>> Can you automate this for 400 applets and ~600 options?
>
> I guess there is no need, what about a description in the beginig like
> "All sizes relate to i686, depending on your target arch this may differ"
> (replace i686 with anything you like)
>
> This would solve 2 problems.
> 1. it is clear what the base is (Kang-Che Sung point 3 is valid)
> 2. in future all claims like "save 200 Bytes (tested on MIPSxyz)" are off 
> limits.
>
What do you mean by "beginning" when it comes to menuconfig interface?

And beside this, there're some more problem that I forgot to mention:
4. The sizes will definitely need to be updated somewhen to reflect the code
   changes, and I won't know how much maintenance burden will there be.
   Sure it can be automated (like size_single_applets.sh), but updating the
   Config.in files will probably make you crazy (sorry Denys Vlasenko).
5. What to do if someone propose a new applet into busybox? He or she couldn't
   have the same environment to measure the size.

For my point no.4, how about this: Instead of putting the sizes in the
Config.in files or source code, make them in an external text file named
"size-ref.txt" (outside menuconfig). Then you can try any automate way to
update the file, or even allow users to measure themselves, with their own
environment.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

About the approximate applet sizes in menuconfig

[Kang-Che Sung]

(https://git.busybox.net/busybox/commit/?id=4eed2c6c5092ed95b8ee6d994106c54a9fc6ed3e)

I don't like this. I don't like these size info to be put on the titles
of config options. A few reasons:
* It can give a false impression that the size of the final busybox
  binary is the size of these applets summed up, but it's actually not.
  (Some code can share among applets)
* The sizes are CPU architecture dependent.
* How the sizes are measured are even unclear. So, does the size include
  all optional features of the applet, or is it only the basic
  functionality?

If I were to mention the size in menuconfig, I would put in the help
texts of the config options, rather than their titles. And be clear how
the size is measured, for example:

   CONFIG_BINZIP2
   (Adds about 8.8kB on i686, for bzip2 decompression code)
   CONFIG_BZIP2
   (Adds about 18kB on i686, for bzip2 compression and decompression code)

Or alternatively, don't mention the size at all.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: avoid GLIBC'ism %m

[Kang-Che Sung]

On Wed, Jul 19, 2017 at 2:11 AM, Markus Gothe  wrote:
> Actually last time I checked ‘%m’ is POSIX contrary to glibc’s deprecated 
> '%a’.
> However, I agree that it should not be used since at least uClibc can be 
> built without support for it.

How come %m is POSIX when I didn't see any mention of it in this page?
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fprintf.html
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] ash: avoid GLIBC'ism %m

[Kang-Che Sung]

I wonder if there's a better solution to this.
BusyBox has bb_perror_msg() and like. Maybe using these API's are better?

On Mon, Jul 17, 2017 at 4:08 AM, Johannes Schindelin
 wrote:
> GLIBC's printf() family supports the extension where the placeholder %m
> is interpolated to strerror(errno).
>
> This is not portable. So don't use it. (It was only used in ash's source
> code to begin with.)
>
> Signed-off-by: Johannes Schindelin 
> ---
> Published-As: 
> https://github.com/dscho/busybox-w32/releases/tag/busybox-glibc-ism-v1
> Fetch-It-Via: git fetch https://github.com/dscho/busybox-w32 
> busybox-glibc-ism-v1
>  shell/ash.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/shell/ash.c b/shell/ash.c
> index b0c7dac54..e2ff15767 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -3809,8 +3809,10 @@ freejob(struct job *jp)
>  static void
>  xtcsetpgrp(int fd, pid_t pgrp)
>  {
> -   if (tcsetpgrp(fd, pgrp))
> -   ash_msg_and_raise_error("can't set tty process group (%m)");
> +   if (tcsetpgrp(fd, pgrp)) {
> +   const char *err = strerror(errno);
> +   ash_msg_and_raise_error("can't set tty process group (%s)", 
> err);
> +   }
>  }
>
>  /*
> @@ -5343,7 +5345,7 @@ savefd(int from)
> err = newfd < 0 ? errno : 0;
> if (err != EBADF) {
> if (err)
> -   ash_msg_and_raise_error("%d: %m", from);
> +   ash_msg_and_raise_error("%d: %s", from, 
> strerror(errno));
> close(from);
> fcntl(newfd, F_SETFD, FD_CLOEXEC);
> }
> @@ -5358,7 +5360,7 @@ dup2_or_raise(int from, int to)
> newfd = (from != to) ? dup2(from, to) : to;
> if (newfd < 0) {
> /* Happens when source fd is not open: try "echo >&99" */
> -   ash_msg_and_raise_error("%d: %m", from);
> +   ash_msg_and_raise_error("%d: %s", from, strerror(errno));
> }
> return newfd;
>  }
> @@ -5489,7 +5491,7 @@ redirect(union node *redir, int flags)
> /* "echo >&10" and 10 is a fd opened to a sh script? 
> */
> if (is_hidden_fd(sv, right_fd)) {
> errno = EBADF; /* as if it is closed */
> -   ash_msg_and_raise_error("%d: %m", right_fd);
> +   ash_msg_and_raise_error("%d: %s", right_fd, 
> strerror(errno));
> }
> newfd = -1;
> } else {
> @@ -5523,7 +5525,7 @@ redirect(union node *redir, int flags)
> if (newfd >= 0)
> close(newfd);
> errno = i;
> -   ash_msg_and_raise_error("%d: %m", fd);
> +   ash_msg_and_raise_error("%d: %s", fd, 
> strerror(errno));
> /* NOTREACHED */
> }
> /* EBADF: it is not open - good, remember to 
> close it */
>
> base-commit: 68e980545af6a8ffb2980f94a6edac4dd89940f3
> --
> 2.13.3.windows.1.13.gaf0c2223da0
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] make_single_applets: fix ": $((fail++))" expansion error

[Kang-Che Sung]

$((fail++)) is not a required expression in POSIX, and in "dash" it
could produce an error like this:

./make_single_applets.sh: 61: arithmetic expression: expecting primary: 
"fail++"

Replace this with something portable: fail=$((fail+1)) would work.

Signed-off-by: Kang-Che Sung <explore...@gmail.com>
---
 make_single_applets.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/make_single_applets.sh b/make_single_applets.sh
index 8ad7a7406..329a27d32 100755
--- a/make_single_applets.sh
+++ b/make_single_applets.sh
@@ -54,18 +54,18 @@ for app; do
fi
 
if ! yes '' | make oldconfig >busybox_make_${app}.log 2>&1; then
-   : $((fail++))
+   fail=$((fail+1))
echo "Config error for ${app}"
mv .config busybox_config_${app}
elif ! make $makeopts >>busybox_make_${app}.log 2>&1; then
-   : $((fail++))
+   fail=$((fail+1))
grep -i -e error: -e warning: busybox_make_${app}.log
echo "Build error for ${app}"
mv .config busybox_config_${app}
elif ! grep -q '^#define NUM_APPLETS 1$' include/NUM_APPLETS.h; then
grep -i -e error: -e warning: busybox_make_${app}.log
mv busybox busybox_${app}
-   : $((fail++))
+   fail=$((fail+1))
echo "NUM_APPLETS != 1 for ${app}: `cat include/NUM_APPLETS.h`"
mv .config busybox_config_${app}
else
-- 
2.13.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] cat: allow compiling out -n and -b

[Kang-Che Sung]

When these options were introduced in d88f94a5df3a2edb8ba56fab5c13674b452f87ab
it provides no config options to compile them out. Now provide one.

Introduce config FEATURE_CATN.

Signed-off-by: Kang-Che Sung <explore...@gmail.com>
---
 coreutils/cat.c | 57 +
 1 file changed, 37 insertions(+), 20 deletions(-)

diff --git a/coreutils/cat.c b/coreutils/cat.c
index 4d9147f8a..2510ae501 100644
--- a/coreutils/cat.c
+++ b/coreutils/cat.c
@@ -13,6 +13,13 @@
 //config:cat is used to concatenate files and print them to the 
standard
 //config:output. Enable this option if you wish to enable the 'cat' 
utility.
 //config:
+//config:config FEATURE_CATN
+//config:  bool "Enable -n and -b options"
+//config:  default y
+//config:  depends on CAT
+//config:  help
+//config:-n numbers all output lines while -b numbers nonempty output 
lines.
+//config:
 //config:config FEATURE_CATV
 //config:  bool "cat -v[etA]"
 //config:  default y
@@ -27,12 +34,19 @@
 /* BB_AUDIT SUSv3 compliant */
 /* http://www.opengroup.org/onlinepubs/007904975/utilities/cat.html */
 
+//usage:#if ENABLE_FEATURE_CATN || ENABLE_FEATURE_CATV
+//usage:#define cat_trivial_usage
+//usage:   "[-" IF_FEATURE_CATN("nb") IF_FEATURE_CATV("vteA") "] [FILE]..."
+//usage:#else
 //usage:#define cat_trivial_usage
-//usage:   "[-nb"IF_FEATURE_CATV("vteA")"] [FILE]..."
+//usage:   "[FILE]..."
+//usage:#endif
 //usage:#define cat_full_usage "\n\n"
 //usage:   "Print FILEs to stdout\n"
+//usage:   IF_FEATURE_CATN(
 //usage: "\n   -n  Number output lines"
 //usage: "\n   -b  Number nonempty lines"
+//usage:   )
 //usage:   IF_FEATURE_CATV(
 //usage: "\n   -v  Show nonprinting characters as ^x or M-x"
 //usage: "\n   -t  ...and tabs as ^I"
@@ -147,7 +161,7 @@ int cat_main(int argc UNUSED_PARAM, char **argv)
 
IF_FEATURE_CATV(opt_complementary = "Aetv"; /* -A == -vet */)
/* -u is ignored ("unbuffered") */
-   opts = getopt32(argv, IF_FEATURE_CATV("etvA")"nbu");
+   opts = getopt32(argv, IF_FEATURE_CATV("etvA") IF_FEATURE_CATN("nb") 
"u");
argv += optind;
 
 #if ENABLE_FEATURE_CATV
@@ -157,23 +171,26 @@ int cat_main(int argc UNUSED_PARAM, char **argv)
opts >>= 4;
 #endif
 
-#define CAT_OPT_n (1<<0)
-#define CAT_OPT_b (1<<1)
-#define CAT_OPT_u (1<<2)
-   if (!(opts & (CAT_OPT_n|CAT_OPT_b))) /* no -n or -b */
-   return bb_cat(argv);
+#if ENABLE_FEATURE_CATN
+# define CAT_OPT_n (1<<0)
+# define CAT_OPT_b (1<<1)
+   if (opts & (CAT_OPT_n|CAT_OPT_b)) { /* -n or -b */
+   if (!*argv)
+   *--argv = (char*)"-";
+   ns.width = 6;
+   ns.start = 1;
+   ns.inc = 1;
+   ns.sep = "\t";
+   ns.empty_str = "\n";
+   ns.all = !(opts & CAT_OPT_b); /* -n without -b */
+   ns.nonempty = (opts & CAT_OPT_b); /* -b (with or without -n) */
+   do {
+   print_numbered_lines(, *argv);
+   } while (*++argv);
+   fflush_stdout_and_exit(EXIT_SUCCESS);
+   }
+   opts >>= 2;
+#endif
 
-   if (!*argv)
-   *--argv = (char*)"-";
-   ns.width = 6;
-   ns.start = 1;
-   ns.inc = 1;
-   ns.sep = "\t";
-   ns.empty_str = "\n";
-   ns.all = !(opts & CAT_OPT_b); /* -n without -b */
-   ns.nonempty = (opts & CAT_OPT_b); /* -b (with or without -n) */
-   do {
-   print_numbered_lines(, *argv);
-   } while (*++argv);
-   fflush_stdout_and_exit(EXIT_SUCCESS);
+   return bb_cat(argv);
 }
-- 
2.13.0

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Need Help with BusyBox Configuration

[Kang-Che Sung]

On Tue, Jul 11, 2017 at 7:42 PM, Siddharth Muralee
 wrote:
> Hey there,
>   I am relatively new to busybox and I am trying to compile a 32bit Linux
> kernel of 2.6.32 and I am not able to compile busybox for 32bit.
> Currently Busybox binary is of
> busybox: ELF 64-bit LSB executable, x86-64, version 1 (GNU/Linux),
> statically linked, for GNU/Linux 2.6.32
>
> I tried using a lot of things in the make like ARCH, CFLAGS, LDFLAGS, TARGET
> etc. And I have not been able to make any progress can you please guide me
> through the same
>

You better tell us the compiler tools you use, as well as the flags you have
tried so far, in detail. Otherwise we can't help you as we don't have enough
information from you.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/1] makedevs: set path size to match linux

[Kang-Che Sung]

On Thu, Jul 6, 2017 at 8:04 AM, Denys Vlasenko <vda.li...@googlemail.com> wrote:
> On Tue, Jun 27, 2017 at 3:20 PM, Kang-Che Sung <explore...@gmail.com> wrote:
>> Hello. Can I suggest another way?
>> How about modifying the `line` buffer so it can be used directly as the name,
>> and avoid the malloc problem or STRINGIFY(PATH_MAX) altogether.
>>
>> My suggestion (warning, this code is not tested):
>>
>> --- a/miscutils/makedevs.c
>> +++ b/miscutils/makedevs.c
>> @@ -209,23 +209,27 @@ int makedevs_main(int argc UNUSED_PARAM, char **argv)
>>   unsigned increment = 0;
>>   unsigned start = 0;
>>   char name[41];
>> + int name_len;
>>   char user[41];
>>   char group[41];
>> - char *full_name = name;
>> + char *full_name;
>>   uid_t uid;
>>   gid_t gid;
>>
>>   linenum = parser->lineno;
>>
>> - if ((2 > sscanf(line, "%40s %c %o %40s %40s %u %u %u %u %u",
>> - name, , , user, group,
>> + if ((1 > sscanf(line, "%*s%n %c %o %40s %40s %u %u %u %u %u",
>> + _len, , , user, group,
>>   , , , , ))
>> + || (PATH_MAX > name_len + 1)
>>   || ((unsigned)(major | minor | start | count | increment) > 255)
>>   ) {
>>   bb_error_msg("invalid line %d: '%s'", linenum, line);
>>   ret = EXIT_FAILURE;
>>   continue;
>>   }
>> + line[name_len] = '\0';
>> + full_name = line;
>
> Good idea!
>
> Applied, thanks!

Wait. I was not expecting the patch be applied directly. I was expecting it
be tested first.
I think my code has a bug:
If the line begins with whitespaces, the initial whitespaces will be included
in `full_name` while the original code won't.
Denys Vlasenko, are you willing to look at this problem?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 1/1] makedevs: set path size to match linux

[Kang-Che Sung]

On Tue, Jun 27, 2017 at 3:53 PM, walter harms  wrote:
>
> Am 27.06.2017 05:31, schrieb Baruch Siach:
>>
>> On Mon, Jun 26, 2017 at 08:45:42PM -0500, Matthew Weber wrote:
>>> On Mon, Jun 26, 2017 at 7:36 PM, Emmanuel Deloget  wrote:
 On Mon, Jun 26, 2017 at 11:23 PM, Matthew Weber
  wrote:
> On Mon, Jun 26, 2017 at 3:55 PM, Baruch Siach  wrote:
>> On Mon, Jun 26, 2017 at 03:33:09PM -0500, Matt Weber wrote:
>>> From: Jared Bents 
>>>
>>> Update to increase the pathname limit to the
>>> linux limit of 4096 characters.
>>>
>>> Similar patch:
>>> https://patchwork.openembedded.org/patch/131475/
>>>
>>> Signed-off-by: Jared Bents 
>>> Signed-off-by: Matt Weber 
>>> ---
>>>  miscutils/makedevs.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/miscutils/makedevs.c b/miscutils/makedevs.c
>>> index 9e7ca34..0049edb 100644
>>> --- a/miscutils/makedevs.c
>>> +++ b/miscutils/makedevs.c
>>> @@ -208,7 +208,7 @@ int makedevs_main(int argc UNUSED_PARAM, char
>>> **argv)
>>>   unsigned count = 0;
>>>   unsigned increment = 0;
>>>   unsigned start = 0;
>>> - char name[41];
>>> + char name[4096];
>>
>> Why not use PATH_MAX here?
>
> Agree, that would be cleaner.  Will submit v2 after some testing.
>
> That still leaves a hardcoded value in the sscanf  of 4095
> should I add a comment to the affect we're assuming PATH_MAX is at
> least 4096?  Maybe a check is also needed?


 Alternatively you may use the m modifier, when implemented, to 
 auto-allocate
 the name pointer. This way you don't have to hardcode anything in the
 sscanf(), you let the library for the job for you. Later, you can check the
 string length.

 Such a change would induce an allocation, a free but will also reduce stack
 usage.

>>>
>>> If we want to keep it all static, another option would be to stringify
>>> that define.
>>> (Courtesy Jared for this idea)
>>>
>>> + #define STRINGIFY(x) STRINGIFY2(x)
>>> + #define STRINGIFY2(x) #x
>>> .
>>> - if ((2 > sscanf(line, "%40s %c %o %40s %40s %u %u %u %u %u",
>>> + if ((2 > sscanf(line, "%" STRINGIFY(PATH_MAX) "s %c %o %40s %40s %u
>>> %u %u %u %u",
>>
>> You need 'STRINGIFY(PATH_MAX-1)'. I'm not sure this does what you mean.
>>
>
> May i ask "what is the problem (beyond truncation) what we want to solve" ?
> It there a need for "unlimited" length ?
>
> Independed of that it would be possible the use the %m modifier for scanf.
> (and break older verions of glibc).
> see: 
> https://stackoverflow.com/questions/38685724/difference-between-ms-and-s-scanf
>

Hello. Can I suggest another way?
How about modifying the `line` buffer so it can be used directly as the name,
and avoid the malloc problem or STRINGIFY(PATH_MAX) altogether.

My suggestion (warning, this code is not tested):

--- a/miscutils/makedevs.c
+++ b/miscutils/makedevs.c
@@ -209,23 +209,27 @@ int makedevs_main(int argc UNUSED_PARAM, char **argv)
  unsigned increment = 0;
  unsigned start = 0;
  char name[41];
+ int name_len;
  char user[41];
  char group[41];
- char *full_name = name;
+ char *full_name;
  uid_t uid;
  gid_t gid;

  linenum = parser->lineno;

- if ((2 > sscanf(line, "%40s %c %o %40s %40s %u %u %u %u %u",
- name, , , user, group,
+ if ((1 > sscanf(line, "%*s%n %c %o %40s %40s %u %u %u %u %u",
+ _len, , , user, group,
  , , , , ))
+ || (PATH_MAX > name_len + 1)
  || ((unsigned)(major | minor | start | count | increment) > 255)
  ) {
  bb_error_msg("invalid line %d: '%s'", linenum, line);
  ret = EXIT_FAILURE;
  continue;
  }
+ line[name_len] = '\0';
+ full_name = line;

  gid = (*group) ? get_ug_id(group, xgroup2gid) : getgid();
  uid = (*user) ? get_ug_id(user, xuname2uid) : getuid();
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ash buildin "read" interrupted on SIGCHLD

[Kang-Che Sung]

On Thu, Jun 1, 2017 at 5:36 PM, Guido Classen  wrote:
> I was curious how to catch SIGCHLD using trap and what behavior we should
> expect. Could you also provide a test for trap in conjunction with SIGCHLD.

I was about to ask the same question, until I realize that the SIGCHLD
handling behavior with respect to `trap` command is never standardized
and varies among shells.

> Here my tries with results from busybox-git with your fix:
>
> # sleep 1& read x
> => no trap signal handler installed, read waits now forever for input

This is expected behavior in all shells and is included in BusyBox testsuite.

> # trap "echo --TRAP---" CHLD; sleep 1& read x
> --TRAP---
> [1]+  Done   sleep 1
>
> => Okay, read will be interrupted after one second
>(this works now thanks your fixes in signal handling coder.
> It would not work correctly in busybox 1.24 versions)

For `ash` (and `dash` also), this would interrupt `read`.
But for `hush`, (and `ksh93` according to my testing), this will run the
SIGCHLD handler, but after that the `read` command is executed again and it
waits for user input.
(The least desirable behavior I've ever seen is in `bash`, where it _blocks_
SIGCHLD altogether during the `read` command, and executes the handler
only after the `read` is finished.)

> # sleep 1& read x
> --TRAP---
> [1]+  Done   sleep 1
>
> => Okay, trap handler still installed, same in subsequent commands

A shell's trap handler is not reset after executing the handler, unless you tell
to reset it within the handler. Required by POSIX, actually.

(http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#trap
"Traps shall remain in place for a given shell until explicitly
changed with another trap command.")

> # trap '' CHLD
> # sleep 1& read x
> [1]+  Done   sleep 1
>
> => Is this correct? Set SIGCHLD to ignore, read still will be
>interrupted after one second.

I'm not sure about this. Sorry.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: tree

[Kang-Che Sung]

On Wed, May 3, 2017 at 6:00 AM, Jethro Tull  wrote:
> why tree is not in busybox?

Short answer: Because no one has been implementing it in Busybox yet.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 3/3] ash: exec: -a option for setting zeroth arg

[Kang-Che Sung]

Back in January 2017, a person named Patrick Pief has suggested a similar patch
to add "exec -a" support:

http://lists.busybox.net/pipermail/busybox/2017-January/085146.html

Is your patch same as his or is there any difference?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: A good scripting language for busybox?

[Kang-Che Sung]

On Fri, Mar 17, 2017 at 11:20 PM, Pavel Aronsky  wrote:
> Apologies for maybe a wild or off-topic question.
> After dealing with quite a few products with busybox and its ash shell used
> as the primary scripting language, I'd like to ask you, busybox experts:
> what are alternatives?
>
> This page: https://busybox.net/tinyutils.html  - mentions Lua and
> Micro-perl. I'd rather perfer a small subset of Python, but cold not find
> one after a day of googling (this is surprising. I've been sure such things
> exists).
>
> However my search hit one interesting Javascript engine named Duktape
> (duktape.org).
>
> Javascript looks almost as good as Python for me, it is popular and should
> be familiar to new developers. Lua is less familiar, but much better for
> writing moderately simple app logic than the *dreadful* shell language.
>
> So the question: how feasible would be inclusion of Lua or Javascript into
> BB, as option for systems where one of these languages will be heavy used?
>
> As "plan B": has anyone seen (or thought of) a FFI interface for BB that
> would allow to call shared libraries written in C, from ash?
>

BusyBox isn't a project that would make a "scripting language" or a
"programming environment" for you. BusyBox is a set of common Unix utilities
fused in one binary that can be used on small or embedded OS distributions.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox