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:", &prompt);
>> + 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: [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 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: 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: 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 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: [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: [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] 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] 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 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] 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 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 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, &statbuf) != 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 sea

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(&loopinfo, 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, &loopinfo);
> -   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(&loopinfo2, 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, &loopinfo);
> +   loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> +   rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo2);
> }
> 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, &loopinfo);
> +   rc = ioctl(lfd, BB_LOOP_GET_STATUS, &tmp);
>
> /* 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_in

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(&config, 0, sizeof(config));
> +   config.fd = ffd;
> +   memcpy(&config.info, loopinfo, sizeof(config.info));
> +
> +   rc = ioctl(lfd, LOOP_CONFIGURE, &config);
> +   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

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, &loopinfo);
> >  > +   loopinfo2.lo_flags -= BB_LO_FLAGS_AUTOCLEAR;
> >  > +   rc = ioctl(lfd, BB_LOOP_SET_STATUS, &loopinfo2);
> >  > }
> >  > 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 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] 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: 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] 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: [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] 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", &st) != 0 || !S_ISREG(st.st_mode)) {
> -   bb_error_msg_and_die("'%s' is not a regular file",
"/init");
> -   }
> statfs("/", &stfs); // 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] 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: 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: 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]

(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: [PATCH] mdev: add NULL-checks in clean_up_cur_rule()

[Kang-Che Sung]

Maks Mishin  於 2024年8月19日 星期一寫道：
> The function clean_up_cur_rule() calls in the loop, which can lead to
> double-free of pointers `G.cur_rule.envvar` and `G.cur_rule.ren_mov`.
> Added NULL checks and NULL assignment after free for correct checks.
>

> -   free(G.cur_rule.envvar);
> -   free(G.cur_rule.ren_mov);
> +   if (G.cur_rule.envvar != NULL) {
> +   free(G.cur_rule.envvar);
> +   G.cur_rule.envvar = NULL;
> +   }
> +   if (G.cur_rule.ren_mov != NULL) {
> +   free(G.cur_rule.ren_mov);
> +   G.cur_rule.ren_mov = NULL;
> +   }
> +

libc free() function should do no-op if the argument is NULL. Thus the
check for NULL conditionals may be removed.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] utils: Remove always true checks

[Kang-Che Sung]

Maks Mishin  於 2024年8月23日 星期五寫道：
> Expression 'res <= 4294967295UL' is always true , which may be caused
> by a logical error: 'res' has a type 'unsigned long' with minimum value
'0'
> and a maximum value '4294967295'
>
> Found by the static analyzer Svace.
>
> Signed-off-by: Maks Mishin 
> ---
>  networking/libiproute/utils.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/networking/libiproute/utils.c b/networking/libiproute/utils.c
> index 3cce4a06e..9e8600f34 100644
> --- a/networking/libiproute/utils.c
> +++ b/networking/libiproute/utils.c
> @@ -42,7 +42,7 @@ unsigned FAST_FUNC get_unsigned(char *arg, const char
*errmsg)
> if (*arg) {
> res = strtoul(arg, &ptr, 0);
>  //FIXME: "" will be accepted too, is it correct?!
> -   if (!*ptr && res <= UINT_MAX) {
> +   if (!*ptr) {
> return res;
> }
> }
> @@ -57,7 +57,7 @@ uint32_t FAST_FUNC get_u32(char *arg, const char
*errmsg)
> if (*arg) {
> res = strtoul(arg, &ptr, 0);
>  //FIXME: "" will be accepted too, is it correct?!
> -   if (!*ptr && res <= 0xUL) {
> +   if (!*ptr) {
> return res;
> }
> }
> --

No, I don't think removal is the right way. `long` is 64-bit in 64-bit OS
(the "LP64" data model). You get the always true condition only on 32-bit
systems, or on Windows, which uses LLP64.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Notify when finished

[Kang-Che Sung]

Harkaitz Agirre  於 2024年8月26日 星期一寫道：
> This patch introduces the "notify me when finished" feature in Busybox
Shell.
>
> When executing long-running commands (such as compiling code or performing
> backups), it can be useful to receive notifications when these tasks
finish.
>
> This allows the user to step away from the terminal, perhaps to grab a
coffee.
>
> The need for a notification is usually an afterthought, so it is not
> practical to add a prefix to the commands in order to being notified when
> finished. Instead, it is more convenient to have a mechanism that
automatically
> alerts the user when a command takes longer than expected.
>
> This change makes ash read two variables, TIMED_ALERT and
TIMED_ALERT_SECS.
>
> If a command execution exceeds the time specified in $TIMED_ALERT_SECS (by
> default 60 seconds) executes the command in $TIMED_ALERT (if set).
>
> The command in $TIMED_ALERT can consult
$TIMED_COMMAND,$TIMED_COMMAND_DURATION
> and $? to prepare the message to send.
>
> One example:
>
> $ export TIMED_ALERT='notify-send "Command finished" "Command
$TIMED_COMMAND finished in $TIMED_COMMAND_DURATION seconds with exit status
$?"'
> $ export TIMED_ALERT_SECS=10
>

My personal comments on this:

1. Does any other core utility or shell ever implement this feature? If
not, it's unlikely that busybox should be the first to support this feature
either.

2. My concern is on the future interoperability with other Unix shells, as
you would now reserve `TIMED_ALERT*` and `TIMED_COMMAND*` environment
variables for this BusyBox-exclusive feature. Better not do this unless
there is no other option available.

3. As other people have suggested: this feature can be implemented as a
separate wrapper script. Making the `TIMED_ALERT` built-in can sometimes be
annoying when user has to run a command that lasts a long duration simply
because it is interactive (e.g. `top(1)`) or it is meant to monitor
something in the back . And your `TIMED_ALERT` feature doesn't seem to
exempt these commands from alerting.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 1/2] doc: Update filenames in keep_data_small.txt

[Kang-Che Sung]

The filenames in docs/keep_data_small.txt are a little bit outdated.
It's better to change it to the current name.

decompress_unzip.c -> decompress_gunzip.c
(since commit 774bce8e8ba1e424c953e8f13aee8f0778c8a911)
libbb/messages.c -> libbb/ptr_to_globals.c
(since commit 574f2f43948bb21d6e4187936ba5a5afccba25f6)

Signed-off-by: Kang-Che Sung 
---
 docs/keep_data_small.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/keep_data_small.txt b/docs/keep_data_small.txt
index 3ced1a6..218d4f2 100644
--- a/docs/keep_data_small.txt
+++ b/docs/keep_data_small.txt
@@ -59,7 +59,7 @@ wait
  Example 1

 One example how to reduce global data usage is in
-archival/libarchive/decompress_unzip.c:
+archival/libarchive/decompress_gunzip.c:

 /* This is somewhat complex-looking arrangement, but it allows
  * to place decompressor state either in bss or in
@@ -87,7 +87,7 @@ take a look at archival/gzip.c. Here all global data
is replaced by
 single global pointer (ptr_to_globals) to allocated storage.

 In order to not duplicate ptr_to_globals in every applet, you can
-reuse single common one. It is defined in libbb/messages.c
+reuse single common one. It is defined in libbb/ptr_to_globals.c
 as struct globals *const ptr_to_globals, but the struct globals is
 NOT defined in libbb.h. You first define your own struct:

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

[PATCH 2/2] doc: Update bb_common_bufsiz1 usage

[Kang-Che Sung]

The commit e6a2f4cc5a47d3022bdf5ca2cacbaa5a8c5baf7a ("libbb: make
bb_common_bufsiz1 1 kbyte, add capability to use bss tail for it")
changes the usage syntax for bb_common_bufsiz1.
Update doc/keep_data_small.txt to reflect the new usage. The change is
probably not perfect, though.

Signed-off-by: Kang-Che Sung 
---
 docs/keep_data_small.txt | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/docs/keep_data_small.txt b/docs/keep_data_small.txt
index 218d4f2..32f57c3 100644
--- a/docs/keep_data_small.txt
+++ b/docs/keep_data_small.txt
@@ -124,11 +124,22 @@ its needs. Library functions are prohibited from using it.

 'G.' trick can be done using bb_common_bufsiz1 instead of malloced buffer:

-#define G (*(struct globals*)&bb_common_bufsiz1)
+#include "common_bufsiz.h"
+#define G (*(struct globals*)bb_common_bufsiz1)
+
+This bb_common_bufsiz1 needs to be initialized before use, by calling
+setup_common_bufsiz(). It is recommended that you define the INIT_G()
+macro:
+
+#define INIT_G() do { setup_common_bufsiz(); } while (0)
+
+And call INIT_G() before you use it (e.g. in _main() function).

 Be careful, though, and use it only if globals fit into bb_common_bufsiz1.
-Since bb_common_bufsiz1 is BUFSIZ + 1 bytes long and BUFSIZ can change
-from one libc to another, you have to add compile-time check for it:
+bb_common_bufsiz1 is COMMON_BUFSIZE bytes long. (Currently
+COMMON_BUFSIZE=1024 but it may change in future releases. Read
+scripts/generate_BUFSIZ.sh code for definition.) You may need to add
+compile-time check for it:

 if (sizeof(struct globals) > sizeof(bb_common_bufsiz1))
  BUG__globals_too_big();
-- 
2.9.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

BusyBox 'ash' should reset SIGQUIT action of its children (Bug)

[Kang-Che Sung]

I recently discovered this bug, but since BusyBox website is offline
for now, I think I'll report here first.

Test case (try on a freshly logged-in tty where busybox ash isn't the
default shell):

$ bash
$ exec bash
$ yes >/dev/null
$ # (Press Ctrl+\ )
$ dash
$ exec dash
$ yes >/dev/null
$ # (Press Ctrl+\ )
$ busybox ash
$ exec busybox ash
$ yes >/dev/null
$ # (Press Ctrl+\ )

Actual result: The last `yes` command (the one started after `exec
busybox ash`) doesn't die.

Expected result: All three `yes` commands should die with SIGQUIT.

bash and dash both show the same behavior that the commands we start in
them can get killed with SIGQUIT. But BusyBox ash shows a wrong
behavior that the ignore action of SIGQUIT get carried to child
processes while it shouldn't.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] docs: Update filenames in keep_data_small.txt

[Kang-Che Sung]

(Because I didn't see this one applied on the git master when
Denys Vlasenko's mail said it has been applied, so pardon me to propose
this patch again.)

The filenames in docs/keep_data_small.txt are a little bit outdated.
It's better to change it to the current name.

decompress_unzip.c -> decompress_gunzip.c
(since commit 774bce8e8ba1e424c953e8f13aee8f0778c8a911)
libbb/messages.c -> libbb/ptr_to_globals.c
(since commit 574f2f43948bb21d6e4187936ba5a5afccba25f6)

Signed-off-by: Kang-Che Sung 
---
 docs/keep_data_small.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/keep_data_small.txt b/docs/keep_data_small.txt
index 3ced1a6..218d4f2 100644
--- a/docs/keep_data_small.txt
+++ b/docs/keep_data_small.txt
@@ -59,7 +59,7 @@ wait
  Example 1

 One example how to reduce global data usage is in
-archival/libarchive/decompress_unzip.c:
+archival/libarchive/decompress_gunzip.c:

 /* This is somewhat complex-looking arrangement, but it allows
  * to place decompressor state either in bss or in
@@ -87,7 +87,7 @@ take a look at archival/gzip.c. Here all global data
is replaced by
 single global pointer (ptr_to_globals) to allocated storage.

 In order to not duplicate ptr_to_globals in every applet, you can
-reuse single common one. It is defined in libbb/messages.c
+reuse single common one. It is defined in libbb/ptr_to_globals.c
 as struct globals *const ptr_to_globals, but the struct globals is
 NOT defined in libbb.h. You first define your own struct:

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

Re: Altscreens for less and more

[Kang-Che Sung]

On Mon, Sep 5, 2016 at 3:43 PM, Cág  wrote:

> Forgot to mention: I booted into Debian, opened my st terminal
> that *has* support for altscreen (in config.h you can disable it though)
> and tried both BusyBox and Coreutils less(1). Coreutils' one opened
> an altscreen, BusyBox' one didn't.
> ..

Um... Technically GNU's less is not part of coreutils package. It's a
separate project.
(With dedicated homepages:

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

Re: [PATCH] libbb: More informative "short write" error

[Kang-Che Sung]

On Tue, Sep 6, 2016 at 5:30 AM, Denys Vlasenko  wrote:
> On Mon, Sep 5, 2016 at 10:46 PM, Tito  wrote:
>>
>> Hi,
>> isn't "short write" redundant?
>
> No, it is not: it says that some bytes were written.
> As opposed to the situation when no bytes were written,
> error happened right away.

Um, excuse me, but may I suggest a more informative message than "short write"?
"Short write" is a little bit unclear.

How about saying "data partially written"?
Or "partial data" or "partially done" for short.

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

Re: possible bug in 'sum' command, with a demo

[Kang-Che Sung]

On Thu, Sep 22, 2016 at 11:57 PM, Anne Salemme  wrote:
>
>   I am using BusyBox version v1.22.1 inside MobaXterm, and came across what 
> looks like a bug in the ‘sum’ command…maybe not a bug, but definitely not 
> expected…here is a little demo for you…thanks, I love BusyBox!
>
> Demo shows that given two files with different contents, ‘sum’ gives them an 
> identical checksum, but ‘cksum’ gives them different checksums (as expected).

> [asalemme.asalemme-win7] ➤ ls -l
> total 1
> -rw-r--r--1 asalemme UsersGrp 8 Sep 22 10:27 demo-107108
> -rw-r--r--1 asalemme UsersGrp 8 Sep 22 10:27 demo-145146
>
> [asalemme.asalemme-win7] ➤ sum *
> 59944 1 demo-107108
> 59944 1 demo-145146
>
> [asalemme.asalemme-win7] ➤ cksum *
> 2984653705 8 demo-107108
> 1560277601 8 demo-145146
>
> [asalemme.asalemme-win7] ➤ od -c demo-107108
> 000   1   0   7   1   0   8  \n  \n
> 010
>
> [asalemme.asalemme-win7] ➤ od -c demo-145146
> 000   1   4   5   1   4   6  \n  \n
> 010

I can't reproduce the same result in the latest BusyBox version from git.
Perhaps this has been fixed already?

Here is what I get:

$ printf '\1\0\7\1\0\10\n\n' | cksum
111069432 8
$ printf '\1\0\7\1\0\10\n\n' | sum
18961 1
$ printf '\1\0\7\1\0\10\n\n' | ./busybox cksum
111069432 8
$ printf '\1\0\7\1\0\10\n\n' | ./busybox sum
18961 1
$ printf '\1\4\5\1\4\6\n\n' | ./busybox sum
18961 1
$ printf '\1\4\5\1\4\6\n\n' | sum
18961 1
$ printf '\1\4\5\1\4\6\n\n' | ./busybox cksum
3951495440 8
$ printf '\1\4\5\1\4\6\n\n' | cksum
3951495440 8
$ ./busybox | head -n 1
BusyBox v1.26.0.git (2016-09-23 07:33:08 CST) multi-call binary.
$ sum --version | head -n 1
sum (GNU coreutils) 8.21
$ cksum --version | head -n 1
cksum (coreutils) 8.21
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: possible bug in 'sum' command, with a demo

[Kang-Che Sung]

> I can't reproduce the same result in the latest BusyBox version from git.
> Perhaps this has been fixed already?

Oops. My mistake. Used the wrong input. Here's the corrected one:

$ printf '145146\n\n'| ./busybox cksum
1560277601 8
$ printf '145146\n\n'| cksum
1560277601 8
$ printf '145146\n\n'| ./busybox sum
59944 1
$ printf '145146\n\n'| sum
59944 1
$ printf '107108\n\n'| sum
59944 1
$ printf '107108\n\n'| ./busybox sum
59944 1
$ printf '107108\n\n'| cksum
2984653705 8
$ printf '107108\n\n'| ./busybox cksum
2984653705 8
$ ./busybox | head -n 1
BusyBox v1.26.0.git (2016-09-23 07:33:08 CST) multi-call binary.
$ sum --version | head -n 1
sum (GNU coreutils) 8.21
$ cksum --version | head -n 1
cksum (coreutils) 8.21
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Ash wildcard usability bug

[Kang-Che Sung]

On Wed, Oct 26, 2016 at 1:24 AM, Lauri Kasanen  wrote:
>
> Well, I did try to use tab completion: cat < *zip[TAB]
>
> Of course it doesn't work. In my case, I had many files sharing a
> prefix, but only one ending in *zip, so typing that was several
> times faster than letter- tab, oh more letters, tab, oh, more
> letters, tab, more
>

I wonder why you can't implement an easy workaround using ls or something.

filename="`ls *zip | head -n 1`"
nc 10.0.0.1 12345 < "$filename"
do_something_else < "$filename"

Yeah. Just avoid the glob on the redirection and instead get the
filename before that. Any difficulty?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Ash wildcard usability bug

[Kang-Che Sung]

On Wed, Oct 26, 2016 at 5:28 PM, Lauri Kasanen  wrote:
> On Wed, Oct 26, 2016, at 05:44, Kang-Che Sung wrote:
>> I wonder why you can't implement an easy workaround using ls or
>> something.
>>
>> filename="`ls *zip | head -n 1`"
>> nc 10.0.0.1 12345 < "$filename"
>> do_something_else < "$filename"
>>
>> Yeah. Just avoid the glob on the redirection and instead get the
>> filename before that. Any difficulty?
>
> Because typing that takes longer? You're missing the point of an
> usability feature.
>
> - Lauri

If you are arguing about usability, you should assume a case that you
are in an _interactive_ shell. Non-interactive environment is for shell
scripts, and for that portability and robustness will be more important
than what you called "usability".

And for that portability includes compatibility across platforms and
standards compliance (whenever possible). I think since POSIX has
standardized this behavior, and other shells follow, there is not much
point to argue with these further. At least not in BusyBox.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Potential regression with ash "pause()" commit

[Kang-Che Sung]

Hello.

I want to mention that in this commit
"ash: use pause(), not sigsuspend(), in wait builtin"
8f7b0248adca9a88351fd7f3dd208775242f3fe6
there could be a regression.

I recently read about Problems with pause() in glibc manual

and this is why they recommend against using pause(): The signal could
arrive after the (!pending_sig) check but before the pause(), causing
the pause() to miss the signal or hang there indefinitely.

Have Denys Vlasenko thought of that? Or can we be sure that never
happens?

Thanks.

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

[PATCH 1/3] Fix allnoconfig that ash is built in by default.

[Kang-Che Sung]

Change the config order that SH_IS_* and BASH_IS_* will be prompted
after ASH and HUSH. And if CONFIG_ASH=n, SH_IS_* choice will default to
SH_IS_NONE.

Signed-off-by: Kang-Che Sung 
---
 shell/Config.src | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/shell/Config.src b/shell/Config.src
index 7f5f67050..794f9985a 100644
--- a/shell/Config.src
+++ b/shell/Config.src
@@ -5,10 +5,12 @@

 menu "Shells"

+INSERT

 choice
  prompt "Choose which shell is aliased to 'sh' name"
- default SH_IS_ASH
+ default SH_IS_ASH if ASH
+ default SH_IS_NONE
  help
   Choose which shell you want to be executed by 'sh' alias.
   The ash shell is the most bash compatible and full featured one.
@@ -55,10 +57,6 @@ config BASH_IS_NONE

 endchoice

-
-INSERT
-
-
 config FEATURE_SH_MATH
  bool "POSIX math support"
  default y
-- 
2.11.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 2/3] Explicltly group "ash options" and "hush options"

[Kang-Che Sung]

This tries to workaround a menuconfig bug that all ash options are no
longer indented under "ash" after SH_IS_ASH becomes independently
selectable. Also allows "depends on ASH || SH_IS_ASH || BASH_IS_ASH"
lines and such be grouped into one place.

Signed-off-by: Kang-Che Sung 
---
 shell/Config.src |  9 -
 shell/ash.c  | 20 +---
 shell/hush.c | 16 +---
 3 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/shell/Config.src b/shell/Config.src
index 794f9985a..1fb05fab6 100644
--- a/shell/Config.src
+++ b/shell/Config.src
@@ -57,10 +57,12 @@ config BASH_IS_NONE

 endchoice

+menu "Options common to all shells"
+ depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
+
 config FEATURE_SH_MATH
  bool "POSIX math support"
  default y
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   Enable math support in the shell via $((...)) syntax.

@@ -76,14 +78,12 @@ config FEATURE_SH_MATH_64
 config FEATURE_SH_EXTRA_QUIET
  bool "Hide message on interactive shell startup"
  default y
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   Remove the busybox introduction when starting a shell.

 config FEATURE_SH_STANDALONE
  bool "Standalone shell"
  default n
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   This option causes busybox shells to use busybox applets
   in preference to executables in the PATH whenever possible. For
@@ -116,7 +116,6 @@ config FEATURE_SH_STANDALONE
 config FEATURE_SH_NOFORK
  bool "Run 'nofork' applets directly"
  default n
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   This option causes busybox shells to not execute typical
   fork/exec/wait sequence, but call _main directly,
@@ -134,11 +133,11 @@ config FEATURE_SH_NOFORK
 config FEATURE_SH_HISTFILESIZE
  bool "Use $HISTFILESIZE"
  default y
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   This option makes busybox shells to use $HISTFILESIZE variable
   to set shell history size. Note that its max value is capped
   by "History size" setting in library tuning section.

+endmenu

 endmenu
diff --git a/shell/ash.c b/shell/ash.c
index 430e42a7b..a5a94a18d 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -26,17 +26,18 @@
 //config:  shell (by Herbert Xu), which was created by porting the 'ash' shell
 //config:  (written by Kenneth Almquist) from NetBSD.
 //config:
+//config:menu "ash options"
+//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
+//config:
 //config:config ASH_OPTIMIZE_FOR_SIZE
 //config: bool "Optimize for size instead of speed"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Compile ash for reduced size at the price of speed.
 //config:
 //config:config ASH_INTERNAL_GLOB
 //config: bool "Use internal glob() implementation"
 //config: default y # Y is bigger, but because of uclibc glob() bug,
let Y be default for now
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Do not use glob() function from libc, use internal implementation.
 //config:  Use this if you are getting "glob.h: No such file or directory"
@@ -45,7 +46,6 @@
 //config:config ASH_RANDOM_SUPPORT
 //config: bool "Pseudorandom generator and $RANDOM variable"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Enable pseudorandom generator and dynamic variable "$RANDOM".
 //config:  Each read of "$RANDOM" will generate a new pseudorandom value.
@@ -56,7 +56,6 @@
 //config:config ASH_EXPAND_PRMT
 //config: bool "Expand prompt string"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  "PS#" may contain volatile content, such as backquote commands.
 //config:  This option recreates the prompt string from the environment
@@ -65,70 +64,60 @@
 //config:config ASH_BASH_COMPAT
 //config: bool "bash-compatible extensions"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Enable bash-compatible extensions.
 //config:
 //config:config ASH_IDLE_TIMEOUT
 //config: bool "Idle timeout variable"
 //config: default n
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Enables bash-like auto-logout after $TMOUT seconds of idle time.
 //config:
 //config:config ASH_JOB_CONTROL
 //config: bool "Job control"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Enable job control in the ash shell.
 //config:
 //config:config ASH_ALIAS
 //config: bool &q

[PATCH 3/3] Clarify help text of CONFIG_{SH,BASH}_IS_* options.

[Kang-Che Sung]

Mention the behavior if user selects CONFIG_SH_IS_ASH but not
CONFIG_ASH. We will be explicit that invocations like "busybox ash"
will not work for such configuration.

Also clarify help text of CONFIG_BASH_IS_* that bash compatibility in
ash is not complete. (It shouldn't be anyway - ash can't support every
bash quirk out there.)

Signed-off-by: Kang-Che Sung 
---
 shell/Config.src | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/shell/Config.src b/shell/Config.src
index 1fb05fab6..997f42f91 100644
--- a/shell/Config.src
+++ b/shell/Config.src
@@ -19,12 +19,24 @@ choice
 config SH_IS_ASH
  depends on !NOMMU
  bool "ash"
+ help
+  Choose ash to be the shell executed by 'sh' name.
+  The ash code will be built into busybox. If you didn't say Y to the
+  "ash" choice (CONFIG_ASH) above, this shell may only be invoked by
+  the name 'sh' (and not 'ash').

 config SH_IS_HUSH
  bool "hush"
+ help
+  Choose hush to be the shell executed by 'sh' name.
+  The hush code will be built into busybox. If you didn't say Y to the
+  "hush" choice (CONFIG_HUSH) above, this shell may only be invoked by
+  the name 'sh' (and not 'hush').

 config SH_IS_NONE
  bool "none"
+ help
+  Do not support 'sh' applet name in busybox.

 endchoice

@@ -33,7 +45,8 @@ choice
  default BASH_IS_NONE
  help
   Choose which shell you want to be executed by 'bash' alias.
-  The ash shell is the most bash compatible and full featured one.
+  The ash shell is the most bash compatible and full featured one,
+  although not complete.

   Note that selecting this option does not switch on any bash
   compatibility code. It merely makes it possible to install
@@ -48,12 +61,25 @@ choice
 config BASH_IS_ASH
  depends on !NOMMU
  bool "ash"
+ help
+  Choose ash to be the shell executed by 'bash' name.
+  The ash code will be built into busybox. If you didn't say Y to the
+  "ash" choice (CONFIG_ASH) above, this shell may only be invoked by
+  the name 'bash' (and not 'ash').
+

 config BASH_IS_HUSH
  bool "hush"
+ help
+  Choose hush to be the shell executed by 'bash' name.
+  The hush code will be built into busybox. If you didn't say Y to the
+  "hush" choice (CONFIG_HUSH) above, this shell may only be invoked by
+  the name 'bash' (and not 'hush').

 config BASH_IS_NONE
  bool "none"
+ help
+  Do not support 'bash' applet name in busybox.

 endchoice

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

FAST_FUNC not working well with LTO (Link Time Optimization)

[Kang-Che Sung]

Busybox uses FAST_FUNC macro to tweak with IA-32 calling conventions in
order to make the function call slightly smaller or slightly faster.
However, when I experiment with GCC's LTO (Link Time Optimization), I
discovered that FAST_FUNC could hinder LTO's optimization so that the
resulting executable become a few bytes larger (than what is compiled
without FAST_FUNC).

Although I can comment out the FAST_FUNC lines in include/platform.h to
achieve the level of optimization I want, may I suggest a way for user
to disable FAST_FUNC conveniently?

For example, let me specify CONFIG_EXTRA_CFLAGS="-DFAST_FUNC= -flto"
and I can compile with LTO without a source code hack. It seems like
GCC does not yet provide a macro or a way to detect LTO in code, so
this is the best suggestion I could have.

The changes will be something like below. I would like some comments
about this problem and my suggestion. Please?

Kang-Che Sung ("Explorer")

diff --git a/include/platform.h b/include/platform.h
index c987d418c..7e537b950 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -108,13 +108,19 @@
  * and/or smaller by using modified ABI. It is usually only needed
  * on non-static, busybox internal functions. Recent versions of gcc
  * optimize statics automatically. FAST_FUNC on static is required
- * only if you need to match a function pointer's type */
-#if __GNUC_PREREQ(3,0) && defined(i386) /* || defined(__x86_64__)? */
+ * only if you need to match a function pointer's type.
+ * FAST_FUNC may not work well with -flto so allow user to disable this.
+ * (-DFAST_FUNC= ) */
+#ifndef FAST_FUNC
+# if __GNUC_PREREQ(3,0) && defined(i386)
 /* stdcall makes callee to pop arguments from stack, not caller */
-# define FAST_FUNC __attribute__((regparm(3),stdcall))
+#  define FAST_FUNC __attribute__((regparm(3),stdcall))
 /* #elif ... - add your favorite arch today! */
-#else
-# define FAST_FUNC
+/* x86_64 doesn't need this - its ABI can't be tweaked like IA-32 (can't use
+ * stdcall; the ABI uses 6 regparms already). */
+# else
+#  define FAST_FUNC
+# endif
 #endif

 /* Make all declarations hidden (-fvisibility flag only affects definitions) */
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 1/3] Fix allnoconfig that ash is built in by default.

[Kang-Che Sung]

(This mail and patch was sent to busybox mailing list on Dec 25, 2016,
and I'm re-sending again for people to notice.)

Change the config order that SH_IS_* and BASH_IS_* will be prompted
after ASH and HUSH. And if CONFIG_ASH=n, SH_IS_* choice will default to
SH_IS_NONE.

Signed-off-by: Kang-Che Sung 
---
 shell/Config.src | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/shell/Config.src b/shell/Config.src
index 7f5f67050..794f9985a 100644
--- a/shell/Config.src
+++ b/shell/Config.src
@@ -5,10 +5,12 @@

 menu "Shells"

+INSERT

 choice
  prompt "Choose which shell is aliased to 'sh' name"
- default SH_IS_ASH
+ default SH_IS_ASH if ASH
+ default SH_IS_NONE
  help
   Choose which shell you want to be executed by 'sh' alias.
   The ash shell is the most bash compatible and full featured one.
@@ -55,10 +57,6 @@ config BASH_IS_NONE

 endchoice

-
-INSERT
-
-
 config FEATURE_SH_MATH
  bool "POSIX math support"
  default y
--
2.11.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 2/3] Explicltly group "ash options" and "hush options"

[Kang-Che Sung]

(This mail and patch was sent to busybox mailing list on Dec 25, 2016,
and I'm re-sending again for people to notice.)

This tries to workaround a menuconfig bug that all ash options are no
longer indented under "ash" after SH_IS_ASH becomes independently
selectable. Also allows "depends on ASH || SH_IS_ASH || BASH_IS_ASH"
lines and such be grouped into one place.

Signed-off-by: Kang-Che Sung 
---
 shell/Config.src |  9 -
 shell/ash.c  | 20 +---
 shell/hush.c | 16 +---
 3 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/shell/Config.src b/shell/Config.src
index 794f9985a..1fb05fab6 100644
--- a/shell/Config.src
+++ b/shell/Config.src
@@ -57,10 +57,12 @@ config BASH_IS_NONE

 endchoice

+menu "Options common to all shells"
+ depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
+
 config FEATURE_SH_MATH
  bool "POSIX math support"
  default y
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   Enable math support in the shell via $((...)) syntax.

@@ -76,14 +78,12 @@ config FEATURE_SH_MATH_64
 config FEATURE_SH_EXTRA_QUIET
  bool "Hide message on interactive shell startup"
  default y
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   Remove the busybox introduction when starting a shell.

 config FEATURE_SH_STANDALONE
  bool "Standalone shell"
  default n
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   This option causes busybox shells to use busybox applets
   in preference to executables in the PATH whenever possible. For
@@ -116,7 +116,6 @@ config FEATURE_SH_STANDALONE
 config FEATURE_SH_NOFORK
  bool "Run 'nofork' applets directly"
  default n
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   This option causes busybox shells to not execute typical
   fork/exec/wait sequence, but call _main directly,
@@ -134,11 +133,11 @@ config FEATURE_SH_NOFORK
 config FEATURE_SH_HISTFILESIZE
  bool "Use $HISTFILESIZE"
  default y
- depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH ||
BASH_IS_HUSH
  help
   This option makes busybox shells to use $HISTFILESIZE variable
   to set shell history size. Note that its max value is capped
   by "History size" setting in library tuning section.

+endmenu

 endmenu
diff --git a/shell/ash.c b/shell/ash.c
index 430e42a7b..a5a94a18d 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -26,17 +26,18 @@
 //config:  shell (by Herbert Xu), which was created by porting the 'ash' shell
 //config:  (written by Kenneth Almquist) from NetBSD.
 //config:
+//config:menu "ash options"
+//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
+//config:
 //config:config ASH_OPTIMIZE_FOR_SIZE
 //config: bool "Optimize for size instead of speed"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Compile ash for reduced size at the price of speed.
 //config:
 //config:config ASH_INTERNAL_GLOB
 //config: bool "Use internal glob() implementation"
 //config: default y # Y is bigger, but because of uclibc glob() bug,
let Y be default for now
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Do not use glob() function from libc, use internal implementation.
 //config:  Use this if you are getting "glob.h: No such file or directory"
@@ -45,7 +46,6 @@
 //config:config ASH_RANDOM_SUPPORT
 //config: bool "Pseudorandom generator and $RANDOM variable"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Enable pseudorandom generator and dynamic variable "$RANDOM".
 //config:  Each read of "$RANDOM" will generate a new pseudorandom value.
@@ -56,7 +56,6 @@
 //config:config ASH_EXPAND_PRMT
 //config: bool "Expand prompt string"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  "PS#" may contain volatile content, such as backquote commands.
 //config:  This option recreates the prompt string from the environment
@@ -65,70 +64,60 @@
 //config:config ASH_BASH_COMPAT
 //config: bool "bash-compatible extensions"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Enable bash-compatible extensions.
 //config:
 //config:config ASH_IDLE_TIMEOUT
 //config: bool "Idle timeout variable"
 //config: default n
-//config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Enables bash-like auto-logout after $TMOUT seconds of idle time.
 //config:
 //config:config ASH_JOB_CONTROL
 //config: bool "Job control"
 //config: default y
-//config: depends on ASH || SH_IS_ASH || BA

[PATCH 3/3] Clarify help text of CONFIG_{SH,BASH}_IS_* options.

[Kang-Che Sung]

(This mail and patch was sent to busybox mailing list on Dec 25, 2016,
and I'm re-sending again for people to notice.)

Mention the behavior if user selects CONFIG_SH_IS_ASH but not
CONFIG_ASH. We will be explicit that invocations like "busybox ash"
will not work for such configuration.

Also clarify help text of CONFIG_BASH_IS_* that bash compatibility in
ash is not complete. (It shouldn't be anyway - ash can't support every
bash quirk out there.)

Signed-off-by: Kang-Che Sung 
---
 shell/Config.src | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/shell/Config.src b/shell/Config.src
index 1fb05fab6..997f42f91 100644
--- a/shell/Config.src
+++ b/shell/Config.src
@@ -19,12 +19,24 @@ choice
 config SH_IS_ASH
  depends on !NOMMU
  bool "ash"
+ help
+  Choose ash to be the shell executed by 'sh' name.
+  The ash code will be built into busybox. If you didn't say Y to the
+  "ash" choice (CONFIG_ASH) above, this shell may only be invoked by
+  the name 'sh' (and not 'ash').

 config SH_IS_HUSH
  bool "hush"
+ help
+  Choose hush to be the shell executed by 'sh' name.
+  The hush code will be built into busybox. If you didn't say Y to the
+  "hush" choice (CONFIG_HUSH) above, this shell may only be invoked by
+  the name 'sh' (and not 'hush').

 config SH_IS_NONE
  bool "none"
+ help
+  Do not support 'sh' applet name in busybox.

 endchoice

@@ -33,7 +45,8 @@ choice
  default BASH_IS_NONE
  help
   Choose which shell you want to be executed by 'bash' alias.
-  The ash shell is the most bash compatible and full featured one.
+  The ash shell is the most bash compatible and full featured one,
+  although not complete.

   Note that selecting this option does not switch on any bash
   compatibility code. It merely makes it possible to install
@@ -48,12 +61,25 @@ choice
 config BASH_IS_ASH
  depends on !NOMMU
  bool "ash"
+ help
+  Choose ash to be the shell executed by 'bash' name.
+  The ash code will be built into busybox. If you didn't say Y to the
+  "ash" choice (CONFIG_ASH) above, this shell may only be invoked by
+  the name 'bash' (and not 'ash').
+

 config BASH_IS_HUSH
  bool "hush"
+ help
+  Choose hush to be the shell executed by 'bash' name.
+  The hush code will be built into busybox. If you didn't say Y to the
+  "hush" choice (CONFIG_HUSH) above, this shell may only be invoked by
+  the name 'bash' (and not 'hush').

 config BASH_IS_NONE
  bool "none"
+ help
+  Do not support 'bash' applet name in busybox.

 endchoice

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

Re: [PATCH 1/3] Fix allnoconfig that ash is built in by default.

[Kang-Che Sung]

On Wed, Jan 4, 2017 at 7:01 PM, Denys Vlasenko  wrote:
> With this patch, if I answer "no" to ASH, then all ash config options
> _are skipped_, and then I can specify that "sh" is ash.
> But I can't configure ash features, since they are skipped already
> (config system silently sets them to defaults).
>
> This problem did not exist before the patch.

I have tested with "make oldconfig", and once the SH_IS_ASH is enabled,
"make oldconfig" will eventually restart and prompt for options that were
previously skipped. So ash options _can_ be configured. There was no such
problem you described.

> Your mail agent mangles the patch (tabs->spaces).

Oops. Didn't notice Google Mail did that. I'm resending the patch as
attachment instead.
From 30f4060de0f09cbcbefca461ca3b176eb618c3b0 Mon Sep 17 00:00:00 2001
From: Explorer09 
Date: Sun, 25 Dec 2016 08:32:37 +0800
Subject: [PATCH] Fix allnoconfig that ash is built in by default.

Change the config order that SH_IS_* and BASH_IS_* will be prompted
after ASH and HUSH. And if CONFIG_ASH=n, SH_IS_* choice will default to
SH_IS_NONE.

Signed-off-by: Kang-Che Sung 
---
 shell/Config.src | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/shell/Config.src b/shell/Config.src
index 7f5f67050..794f9985a 100644
--- a/shell/Config.src
+++ b/shell/Config.src
@@ -5,10 +5,12 @@
 
 menu "Shells"
 
+INSERT
 
 choice
 	prompt "Choose which shell is aliased to 'sh' name"
-	default SH_IS_ASH
+	default SH_IS_ASH if ASH
+	default SH_IS_NONE
 	help
 	  Choose which shell you want to be executed by 'sh' alias.
 	  The ash shell is the most bash compatible and full featured one.
@@ -55,10 +57,6 @@ config BASH_IS_NONE
 
 endchoice
 
-
-INSERT
-
-
 config FEATURE_SH_MATH
 	bool "POSIX math support"
 	default y
-- 
2.11.0

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

FAST_FUNC not working well with GCC's LTO

[Kang-Che Sung]

(This mail and patch was sent to busybox mailing list on Dec 25, 2016,
and I'm re-sending again for people to notice.)

Busybox uses FAST_FUNC macro to tweak with IA-32 calling conventions in
order to make the function call slightly smaller or slightly faster.
However, when I experiment with GCC's LTO (Link Time Optimization), I
discovered that FAST_FUNC could hinder LTO's optimization so that the
resulting executable become a few bytes larger (than what is compiled
without FAST_FUNC).

Although I can comment out the FAST_FUNC lines in include/platform.h to
achieve the level of optimization I want, may I suggest a way for user
to disable FAST_FUNC conveniently?

For example, let me specify CONFIG_EXTRA_CFLAGS="-DFAST_FUNC= -flto"
and I can compile with LTO without a source code hack. It seems like
GCC does not yet provide a macro or a way to detect LTO in code, so
this is the best suggestion I could have.

The changes will be something like below. I would like some comments
about this problem and my suggestion. Please?

Kang-Che Sung ("Explorer")



diff --git a/include/platform.h b/include/platform.h
index c987d418c..7e537b950 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -108,13 +108,19 @@
  * and/or smaller by using modified ABI. It is usually only needed
  * on non-static, busybox internal functions. Recent versions of gcc
  * optimize statics automatically. FAST_FUNC on static is required
- * only if you need to match a function pointer's type */
-#if __GNUC_PREREQ(3,0) && defined(i386) /* || defined(__x86_64__)? */
+ * only if you need to match a function pointer's type.
+ * FAST_FUNC may not work well with -flto so allow user to disable this.
+ * (-DFAST_FUNC= ) */
+#ifndef FAST_FUNC
+# if __GNUC_PREREQ(3,0) && defined(i386)
 /* stdcall makes callee to pop arguments from stack, not caller */
-# define FAST_FUNC __attribute__((regparm(3),stdcall))
+#  define FAST_FUNC __attribute__((regparm(3),stdcall))
 /* #elif ... - add your favorite arch today! */
-#else
-# define FAST_FUNC
+/* x86_64 doesn't need this - its ABI can't be tweaked like IA-32 (can't use
+ * stdcall; the ABI uses 6 regparms already). */
+# else
+#  define FAST_FUNC
+# endif
 #endif

 /* Make all declarations hidden (-fvisibility flag only affects definitions) */
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH v2] Explicltly group ash options and hush options

[Kang-Che Sung]

(The patch diff is in mail attachment because I'm not sure if Gmail
likes tab characters in mail messages.)

This would makes all ash options indented inside "ash" in menuconfig.
It appears that menuconfig has a limit at tracking multiple dependency
lines like this (it looks like a "diamond problem" but I'm not sure if
it is):

   ---ASH <--
  /  \   ASH_OPTIMIZE_FOR_SIZE
!NOMMU <-*SH_IS_ASH <[OR] <--ASH_INTERNAL_GLOB
  \  /   ASH_RANDOM_SUPPORT
   ---BASH_IS_ASH <--[...]

The kconfig-language document [1] states that:

> If a menu entry somehow depends on the previous entry, it can be
> made a submenu of it. First, the previous (parent) symbol must be
> part of the dependency list and then one of these two conditions
> must be true:
> - the child entry must become invisible, if the parent is set to 'n'

[BusyBox ash used to satisfy this, but no longer does]

> - the child entry must only be visible, if the parent is visible

[BusyBox ash configs actually satisfy this, but because of
 "diamond" above this might not be easily detected]

So I found out a direct workaround: by making ash options explicitly
depend on !NOMMU, we can tell menuconfig that rule 2 above is satisfied
without any more tracking.

   -
  / \
!NOMMU <-*-ASH < \
  \ \ \ASH_OPTIMIZE_FOR_SIZE
   *---SH_IS_ASH <---[OR]-[AND] <--ASH_INTERNAL_GLOB
\/ ASH_RANDOM_SUPPORT
 --BASH_IS_ASH <-  [...]

So all ash options would now be indented under "ash".

[1] "Documentation/kbuild/kconfig-language.txt" in Linux kernel source

Signed-off-by: Kang-Che Sung 
From abdf0c53e17d8f58336f0558bcee1e2848474901 Mon Sep 17 00:00:00 2001
From: Explorer09 
Date: Thu, 5 Jan 2017 10:55:45 +0800
Subject: [PATCH] Explicltly group "ash options" and "hush options"

This would makes all ash options indented inside "ash" in menuconfig.
It appears that menuconfig has a limit at tracking multiple dependency
lines like this (it looks like a "diamond problem" but I'm not sure if
it is):

   ---ASH <--
  /  \   ASH_OPTIMIZE_FOR_SIZE
!NOMMU <-*SH_IS_ASH <[OR] <--ASH_INTERNAL_GLOB
  \  /   ASH_RANDOM_SUPPORT
   ---BASH_IS_ASH <--[...]

The kconfig-language document [1] states that:

> If a menu entry somehow depends on the previous entry, it can be
> made a submenu of it. First, the previous (parent) symbol must be
> part of the dependency list and then one of these two conditions
> must be true:
> - the child entry must become invisible, if the parent is set to 'n'

[Busybox ash used to satisfy this, but no longer does]

> - the child entry must only be visible, if the parent is visible

[Busybox ash configs actually satisfy this, but because of
 "diamond" above this might not be easily detected]

So I found out a direct workaround: by making ash options explicitly
depend on !NOMMU, we can tell menuconfig that rule 2 above is satisfied
without any more tracking.

   -
  / \
!NOMMU <-*-ASH < \
  \ \ \ASH_OPTIMIZE_FOR_SIZE
   *---SH_IS_ASH <---[OR]-[AND] <--ASH_INTERNAL_GLOB
\/     ASH_RANDOM_SUPPORT
 --BASH_IS_ASH <-  [...]

So all ash options would now be indented under "ash".

[1] "Documentation/kbuild/kconfig-language.txt" in Linux kernel source

Signed-off-by: Kang-Che Sung 
---
 shell/Config.src |  9 -
 shell/ash.c  | 22 +++---
 shell/hush.c | 16 +---
 3 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/shell/Config.src b/shell/Config.src
index 9bd493fed..c08bfc095 100644
--- a/shell/Config.src
+++ b/shell/Config.src
@@ -80,10 +80,12 @@ endchoice
 INSERT
 
 
+comment "Options common to all shells"
+if ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH || BASH_IS_HUSH
+
 config FEATURE_SH_MATH
 	bool "POSIX math support"
 	default y
-	depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUSH || BASH_IS_HUSH
 	help
 	  Enable math support in the shell via $((...)) syntax.
 
@@ -99,14 +101,12 @@ config FEATURE_SH_MATH_64
 config FEATURE_SH_EXTRA_QUIET
 	bool "Hide message on interactive shell startup"
 	default y
-	depends on ASH || HUSH || SH_IS_ASH || BASH_IS_ASH || SH_IS_HUS

[PATCH] dpkg & dpkg_deb: don't depend on FEATURE_SEAMLESS_GZ

[Kang-Che Sung]

CONFIG_DPKG and CONFIG_DPKG_DEB unnecessarily depend on
CONFIG_FEATURE_SEAMLESS_GZ. Worse, it has been using "select" clause,
which would prevent FEATURE_SEAMLESS_GZ from being unselect-able when
using "make config" without a .config file:

$ rm .config
$ make config
[Answer N to almost every boolean choice, and observe that
FEATURE_SEAMLESS_GZ cannot be unselected.]

Because the C code of both dpkg and dpkg-deb are already aware of no-GZ
configuration, it's safe to remove that dependency.
From 12b4368878383c8d55fa13b3817703fda7b60834 Mon Sep 17 00:00:00 2001
From: Explorer09 
Date: Fri, 6 Jan 2017 10:55:41 +0800
Subject: [PATCH] dpkg & dpkg_deb: don't depend on FEATURE_SEAMLESS_GZ

CONFIG_DPKG and CONFIG_DPKG_DEB unnecessarily depend on
CONFIG_FEATURE_SEAMLESS_GZ. Worse, it has been using "select" clause,
which would prevent FEATURE_SEAMLESS_GZ from being unselect-able when
using "make config" without a .config file:

$ rm .config
$ make config
[Answer N to almost every boolean choice, and observe that
FEATURE_SEAMLESS_GZ cannot be unselected.]

Because the C code of both dpkg and dpkg-deb are already aware of no-GZ
configuration, it's safe to remove that dependency.

Signed-off-by: Kang-Che Sung 
---
 archival/dpkg.c | 1 -
 archival/dpkg_deb.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/archival/dpkg.c b/archival/dpkg.c
index f133299e3..22bc6f161 100644
--- a/archival/dpkg.c
+++ b/archival/dpkg.c
@@ -29,7 +29,6 @@
 //config:config DPKG
 //config:	bool "dpkg"
 //config:	default y
-//config:	select FEATURE_SEAMLESS_GZ
 //config:	help
 //config:	  dpkg is a medium-level tool to install, build, remove and manage
 //config:	  Debian packages.
diff --git a/archival/dpkg_deb.c b/archival/dpkg_deb.c
index ebbc7f035..8a97439d8 100644
--- a/archival/dpkg_deb.c
+++ b/archival/dpkg_deb.c
@@ -8,7 +8,6 @@
 //config:config DPKG_DEB
 //config:	bool "dpkg_deb"
 //config:	default y
-//config:	select FEATURE_SEAMLESS_GZ
 //config:	help
 //config:	  dpkg-deb unpacks and provides information about Debian archives.
 //config:
-- 
2.11.0

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

Re: Compiling busybox for ARM-V7

[Kang-Che Sung]

On Fri, Jan 6, 2017 at 12:07 PM, Vered Zvi  wrote:
> I compiled busybox 1.25.1 for ARM-V7 using crosstool-ng toolchain.
>
> I used the commands:
>
> TOOLCHAIN=/home/zvivered/module/CARD/linux4.1.13/toolchain
> make clean
> make ARCH=arm 
> CROSS_COMPILE=$TOOLCHAIN/crosstool/release/bin/arm-cortex_a15-linux-gnueabihf-
>  defconfig
> make ARCH=arm 
> CROSS_COMPILE=$TOOLCHAIN/crosstool/release/bin/arm-cortex_a15-linux-gnueabihf-
> make CONFIG_PREFIX=../../../rootfs install
>
> Then I ran:  file busybox
> and got:
> ./busybox: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically 
> linked (uses shared libs), for GNU/Linux 2.6.32, 
> BuildID[sha1]=4789168febe33cdab87b3d932c0fdbf8d5c73e30, stripped
> [...]
> Also, the busybox utils are running on my centos (64) machine.
> How is it possible that a utility compiled for ARM is running on an x86 
> machine ?

Did you ever check your "arm-cortex_a15-linux-gnueabihf-gcc -v" output?
Look at the "Target:" line, what does it write?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Compiling busybox for ARM-V7

[Kang-Che Sung]

On Fri, Jan 6, 2017 at 7:54 PM, Bob Dunlop  wrote:
> I'm not sure passing the parameters to make like this works,
> they don't get passed to sub processes ?  I've always set them
> in the environment so all children are guaranteed to see them.
>
> I'd stick the commands in a scripts for easy repetition and tweaking.
> Something like this:
>
>   #!/bin/bash
>
>   # Export cross build parameters to environment
>   export ARCH=arm
>   export CROSS_COMPILE=arm-cortex_a15-linux-gnueabihf-
>
>   # Put compiler bin in the front of my PATH (automatically exported)
>   TOOLCHAIN=/home/zvivered/module/CARD/linux4.1.13/toolchain
>   PATH=$TOOLCHAIN/crosstool/release/bin:$PATH
>
>   # Build
>   make clean
>   make defconfig
>   make
>
>   # Use absolute path for install directory
>   make CONFIG_PREFIX=$(pwd)/../../../rootfs install

Makefiles don't always honor environment variables like you expect.
In most cases you are expected to *overwrite* certain variables, for
example $CC to make it work. If unsure, read the makefile code.

Do you know the difference between these two make invocations?

# This overwrites $CC so that make will use your value despite how
# makefile defines the variable $CC.
make CC=arm-cortex_a15-linux-gnueabihf-gcc
# This sets $CC as environment variable before invoking make, but
# makefile may not honor the variable.
CC=arm-cortex_a15-linux-gnueabihf-gcc make

I would recommend against having $ARCH or $CROSS_COMPILE
defined as environment variables. It's better to use overwrite method
for these two. (But it's safe to export $PATH environment variable
because makefiles defining it to something else is unlikely.)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] dpkg & dpkg_deb: don't depend on FEATURE_SEAMLESS_GZ

[Kang-Che Sung]

On Sat, Jan 7, 2017 at 2:31 AM, Jody Bruchon  wrote:
> On 2017-01-06 13:23, Denys Vlasenko wrote:
>>
>> On Fri, Jan 6, 2017 at 4:18 AM, Kang-Che Sung 
>> wrote:
>>>
>>> CONFIG_DPKG and CONFIG_DPKG_DEB unnecessarily depend on
>>> CONFIG_FEATURE_SEAMLESS_GZ. Worse, it has been using "select" clause,
>>> which would prevent FEATURE_SEAMLESS_GZ from being unselect-able when
>>> using "make config" without a .config file:
>>>
>>>  $ rm .config
>>>  $ make config
>>>  [Answer N to almost every boolean choice, and observe that
>>>  FEATURE_SEAMLESS_GZ cannot be unselected.]
>>>
>>> Because the C code of both dpkg and dpkg-deb are already aware of no-GZ
>>> configuration, it's safe to remove that dependency.
>>
>> My understanding is that .deb files usually use .gz compression,
>> and building dpkg without support for .gz results in a useless tool:
>> there are no .deb files which it can process.
>
> Debian packages support gzip or xz for the control.tar file and a variety of
> common compression formats for the data.tar file.

Yes. And my point is that there's no need to force a gz choice for users.
gz could be deprecated. And sometimes a custom distribution may decide
not to gz-compress its .deb packages at all.

I think it will be better to just *recommend* the gz feature instead. Mention
in the help text:

Note that most .deb packages compress their metadata
in gz (control.tar.gz), so you are likely to also enable the
"understand .gz data" feature above (FEATURE_SEAMLESS_GZ).
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] No longer prompt PLATFORM_LINUX option for user.

[Kang-Che Sung]

(Patch included as mail attachment)

With the new "select PLATFORM_LINUX" mechanism
(commit e3b1a1fd28558f7a1b3c0ec33313bedb675be8a1), the PLATFORM_LINUX
option alone no longer has any purpose of changing program behavior or
affecting compiled code. So there is no longer need to prompt user of
this config question.

Signed-off-by: Kang-Che Sung
From 7eb49828448feb5225ed6c210ae962cbdcc484c3 Mon Sep 17 00:00:00 2001
From: Explorer09 
Date: Sat, 7 Jan 2017 15:16:46 +0800
Subject: [PATCH] No longer prompt PLATFORM_LINUX option for user.

With the new "select PLATFORM_LINUX" mechanism
(commit e3b1a1fd28558f7a1b3c0ec33313bedb675be8a1), the PLATFORM_LINUX
option alone no longer has any purpose of changing program behavior or
affecting compiled code. So there is no longer need to prompt user of
this config question.

Signed-off-by: Kang-Che Sung 
---
 Config.in | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/Config.in b/Config.in
index 924a197da..66f7787d2 100644
--- a/Config.in
+++ b/Config.in
@@ -50,17 +50,6 @@ config USE_PORTABLE_CODE
 	  compiler other than gcc.
 	  If you do use gcc, this option may needlessly increase code size.
 
-config PLATFORM_LINUX
-	bool "Enable Linux-specific applets and features"
-	default y
-	help
-	  For the most part, busybox requires only POSIX compatibility
-	  from the target system, but some applets and features use
-	  Linux-specific interfaces.
-
-	  Answering 'N' here will disable such applets and hide the
-	  corresponding configuration options.
-
 config SHOW_USAGE
 	bool "Show applet usage messages"
 	default y
@@ -338,6 +327,17 @@ config FEATURE_HAVE_RPC
 	#  This is automatically selected if any of enabled applets need it.
 	#  You do not need to select it manually.
 
+config PLATFORM_LINUX
+	bool #No description makes it a hidden option
+	default n
+	#help
+	#  For the most part, busybox requires only POSIX compatibility
+	#  from the target system, but some applets and features use
+	#  Linux-specific interfaces.
+	#
+	#  This is automatically selected if any applet or feature requires
+	#  Linux-specific interfaces. You do not need to select it manually.
+
 comment 'Build Options'
 
 config STATIC
-- 
2.11.0

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

[PATCH] Fix code bloat caused by zcat's seamless magic

[Kang-Che Sung]

(I discovered this when personally experimenting the make_single_applets script)

This example single-applet configuration would trigger the bloat:

CONFIG_FEATURE_SEAMLESS_XZ=y
CONFIG_FEATURE_SEAMLESS_LZMA=y
CONFIG_FEATURE_SEAMLESS_BZ2=y
CONFIG_FEATURE_SEAMLESS_GZ=y
CONFIG_BUNZIP2=y
# CONFIG_ZCAT is not set
# All other applets disabled

Here, the resulting "busybox-bunzip2" binary would contain
unpack_gz_stream, unpack_lzma_stream and unpack_xz_stream functions
code. In other words, the gzip, lzma and xz decompressors' code are
linked into the binary unnecessarily.

Fix this by disabling SEAMLESS_MAGIC option flag (setting its value
to 0) when zcat is disabled. This will help the compiler optimize out
bbunpack() and no longer generate open_zipped() function call.

Signed-off-by: Kang-Che Sung 
---
 archival/bbunzip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/archival/bbunzip.c b/archival/bbunzip.c
index 60a837e22..a8d8a9872 100644
--- a/archival/bbunzip.c
+++ b/archival/bbunzip.c
@@ -25,7 +25,7 @@ enum {
  OPT_QUIET  = 1 << 3,
  OPT_DECOMPRESS = 1 << 4,
  OPT_TEST   = 1 << 5,
- SEAMLESS_MAGIC = (1 << 31) * SEAMLESS_COMPRESSION,
+ SEAMLESS_MAGIC = (1 << 31) * (ENABLE_ZCAT && SEAMLESS_COMPRESSION),
 };

 static
@@ -385,7 +385,7 @@ int gunzip_main(int argc UNUSED_PARAM, char **argv)

  return bbunpack(argv, unpack_gz_stream, make_new_name_gunzip,
/*unused:*/ NULL);
 }
-#endif
+#endif /* GUNZIP || ZCAT */


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

[PATCH] Fix ash "kill %1" not working if CONFIG_ASH is disabled

[Kang-Che Sung]

Fix ash "kill %1" not working if CONFIG_ASH is disabled but
ash is launched by 'sh' or 'bash' name

Signed-off-by: Kang-Che Sung 
---
 procps/kill.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/procps/kill.c b/procps/kill.c
index 57a33bcaa..198c78de0 100644
--- a/procps/kill.c
+++ b/procps/kill.c
@@ -285,10 +285,10 @@ int kill_main(int argc UNUSED_PARAM, char **argv)

  /* Looks like they want to do a kill. Do that */
  while (arg) {
-#if ENABLE_ASH || ENABLE_HUSH
+#if ENABLE_ASH || ENABLE_SH_IS_ASH || ENABLE_BASH_IS_ASH
  /*
  * We need to support shell's "hack formats" of
- * " -PRGP_ID" (yes, with a leading space)
+ * " -PGRP_ID" (yes, with a leading space)
  * and " PID1 PID2 PID3" (with degenerate case "")
  */
  while (*arg != '\0') {
-- 
2.11.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Fix ash "kill %1" not working if CONFIG_ASH is disabled

[Kang-Che Sung]

On Mon, Jan 9, 2017 at 10:03 AM, Kang-Che Sung  wrote:
> On Sun, Jan 8, 2017 at 7:40 PM, Xabier Oneca  --  xOneca
>  wrote:
>> Hello Kang-Che,
>>
>>> diff --git a/procps/kill.c b/procps/kill.c
>>> index 57a33bcaa..198c78de0 100644
>>> --- a/procps/kill.c
>>> +++ b/procps/kill.c
>>> @@ -285,10 +285,10 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
>>>
>>>   /* Looks like they want to do a kill. Do that */
>>>   while (arg) {
>>> -#if ENABLE_ASH || ENABLE_HUSH
>>> +#if ENABLE_ASH || ENABLE_SH_IS_ASH || ENABLE_BASH_IS_ASH
>>
>> You dropped ENABLE_HUSH here. Is it intentional?
>>
> hush doesn't have a built-in kill command. Yes, it's intentional.

Correction. Well, hush _used to_ not have a built-in kill command, but
it seems that Denys Vlasenko tried to implement it recently:
https://git.busybox.net/busybox/commit/?id=1125d7d6801940a5218b74c8fd46f1eaa2e4de39
And so my patch got modified by him before getting merged.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] hush: fix config text typo (HUSH_MEMLEAK)

[Kang-Che Sung]

Signed-off-by: Kang-Che Sung 
---
 shell/hush.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shell/hush.c b/shell/hush.c
index 9b62d5c0d..01c334a46 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -291,7 +291,7 @@
 //config:  default n
 //config:  depends on HUSH || SH_IS_HUSH || BASH_IS_HUSH
 //config:  help
-//config:Enable umask builtin in hush.
+//config:Enable memleak builtin in hush.
 //config:
 //config:config MSH
 //config:  bool "msh (deprecated: aliased to hush)"
-- 
2.11.0
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] Allow 'gzip -d' and 'bzip2 -d' without gunzip or bunzip2

[Kang-Che Sung]

Idea copied from the "ip" applet.

(Patch included as mail attachment.)
From 7a9d46fe8cb7acd383059c2496e94c229823c1be Mon Sep 17 00:00:00 2001
From: Kang-Che Sung 
Date: Sun, 8 Jan 2017 14:28:34 +0800
Subject: [PATCH] Allow 'gzip -d' and 'bzip2 -d' without gunzip or bunzip2

Idea copied from the "ip" applet.

Signed-off-by: Kang-Che Sung 
---
 archival/bbunzip.c | 20 ++--
 archival/bzip2.c   | 15 +--
 archival/gzip.c| 24 +---
 3 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/archival/bbunzip.c b/archival/bbunzip.c
index 60a837e22..8ee5a5580 100644
--- a/archival/bbunzip.c
+++ b/archival/bbunzip.c
@@ -12,9 +12,9 @@
 //kbuild:lib-$(CONFIG_LZOPCAT) += bbunzip.o
 //kbuild:lib-$(CONFIG_UNLZOP) += bbunzip.o
 /* bzip2_main() too: */
-//kbuild:lib-$(CONFIG_BZIP2) += bbunzip.o
+//kbuild:lib-$(CONFIG_FEATURE_BZIP2_DECOMPRESS) += bbunzip.o
 /* gzip_main() too: */
-//kbuild:lib-$(CONFIG_GZIP) += bbunzip.o
+//kbuild:lib-$(CONFIG_FEATURE_GZIP_DECOMPRESS) += bbunzip.o
 
 /* Note: must be kept in sync with archival/lzop.c */
 enum {
@@ -197,7 +197,7 @@ int FAST_FUNC bbunpack(char **argv,
 }
 
 #if ENABLE_UNCOMPRESS \
- || ENABLE_BUNZIP2 || ENABLE_BZCAT \
+ || ENABLE_FEATURE_BZIP2_DECOMPRESS \
  || ENABLE_UNLZMA || ENABLE_LZCAT || ENABLE_LZMA \
  || ENABLE_UNXZ || ENABLE_XZCAT || ENABLE_XZ
 static
@@ -295,6 +295,7 @@ int uncompress_main(int argc UNUSED_PARAM, char **argv)
 //config:config GUNZIP
 //config:	bool "gunzip"
 //config:	default y
+//config:	select FEATURE_GZIP_DECOMPRESS
 //config:	help
 //config:	  gunzip is used to decompress archives created by gzip.
 //config:	  You can use the `-t' option to test the integrity of
@@ -303,6 +304,7 @@ int uncompress_main(int argc UNUSED_PARAM, char **argv)
 //config:config ZCAT
 //config:	bool "zcat"
 //config:	default y
+//config:	select FEATURE_GZIP_DECOMPRESS
 //config:	help
 //config:	  Alias to "gunzip -c".
 //config:
@@ -315,9 +317,7 @@ int uncompress_main(int argc UNUSED_PARAM, char **argv)
 
 //applet:IF_GUNZIP(APPLET(gunzip, BB_DIR_BIN, BB_SUID_DROP))
 //applet:IF_ZCAT(APPLET_ODDNAME(zcat, gunzip, BB_DIR_BIN, BB_SUID_DROP, zcat))
-//kbuild:lib-$(CONFIG_GUNZIP) += bbunzip.o
-//kbuild:lib-$(CONFIG_ZCAT) += bbunzip.o
-#if ENABLE_GUNZIP || ENABLE_ZCAT
+#if FEATURE_GZIP_DECOMPRESS
 static
 char* FAST_FUNC make_new_name_gunzip(char *filename, const char *expected_ext UNUSED_PARAM)
 {
@@ -385,7 +385,7 @@ int gunzip_main(int argc UNUSED_PARAM, char **argv)
 
 	return bbunpack(argv, unpack_gz_stream, make_new_name_gunzip, /*unused:*/ NULL);
 }
-#endif
+#endif /* FEATURE_GZIP_DECOMPRESS */
 
 
 /*
@@ -408,6 +408,7 @@ int gunzip_main(int argc UNUSED_PARAM, char **argv)
 //config:config BUNZIP2
 //config:	bool "bunzip2"
 //config:	default y
+//config:	select FEATURE_BZIP2_DECOMPRESS
 //config:	help
 //config:	  bunzip2 is a compression utility using the Burrows-Wheeler block
 //config:	  sorting text compression algorithm, and Huffman coding. Compression
@@ -421,14 +422,13 @@ int gunzip_main(int argc UNUSED_PARAM, char **argv)
 //config:config BZCAT
 //config:	bool "bzcat"
 //config:	default y
+//config:	select FEATURE_BZIP2_DECOMPRESS
 //config:	help
 //config:	  Alias to "bunzip2 -c".
 
 //applet:IF_BUNZIP2(APPLET(bunzip2, BB_DIR_USR_BIN, BB_SUID_DROP))
 //applet:IF_BZCAT(APPLET_ODDNAME(bzcat, bunzip2, BB_DIR_USR_BIN, BB_SUID_DROP, bzcat))
-//kbuild:lib-$(CONFIG_BUNZIP2) += bbunzip.o
-//kbuild:lib-$(CONFIG_BZCAT) += bbunzip.o
-#if ENABLE_BUNZIP2 || ENABLE_BZCAT
+#if FEATURE_BZIP2_DECOMPRESS
 int bunzip2_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int bunzip2_main(int argc UNUSED_PARAM, char **argv)
 {
diff --git a/archival/bzip2.c b/archival/bzip2.c
index 47fa29af3..893060777 100644
--- a/archival/bzip2.c
+++ b/archival/bzip2.c
@@ -19,6 +19,14 @@
 //config:
 //config:	  Unless you have a specific application which requires bzip2, you
 //config:	  should probably say N here.
+//config:
+//config:config FEATURE_BZIP2_DECOMPRESS
+//config:	default y
+//config:	depends on BZIP2 || BUNZIP2 || BZCAT
+//config:	help
+//config:	  Enable -d (--decompress) and -t (--test) options for bzip2.
+//config:	  This will be automatically selected if bunzip2 or bzcat is
+//config:	  enabled.
 
 //applet:IF_BZIP2(APPLET(bzip2, BB_DIR_USR_BIN, BB_SUID_DROP))
 //kbuild:lib-$(CONFIG_BZIP2) += bzip2.o
@@ -28,7 +36,10 @@
 //usage:#define bzip2_full_usage "\n\n"
 //usage:   "Compress FILEs (or stdin) with bzip2 algorithm\n"
 //usage: "\n	-1..9	Compression level"
+//usage:	IF_FEATURE_BZIP2_DECOMPRESS(
 //usage: "\n	-d	Decompress"
+//usage: "\n	-t	Test file integrity"
+//usage:	)
 //usage: "\n	-c	Write to stdout"
 //usage: "\n	-f	Force"
 
@@ -184,8 +195,8 @@ int bzip2_main(int argc UNUSED_PARAM, char **argv)
 
 	opt_complement

[RFC] modprobe-small: optimizations for single applet build

[Kang-Che Sung]

(I'm requesting for a review first because I fear such an aggressive
change could lead to bugs. While I observe the sizes have reduced, I
haven't test the functionality of each applet after that. So please
test before merging.)

Aggressively cut off unneeded code when the relevant applets are not
built.

Correct dependencies of FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE and
FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED.

Don't bother with the '-r' option check if only rmmod is built (assume
true then), or when neither rmmod or mobprobe is built (assume false
then).

Size comparison before and after the change (single applet
configuration):

 textdata bss dec hex filename
34778 946 112   358368bfc old/busybox_DEPMOD
34151 946 112   352098989 new/busybox_DEPMOD

34903 946 112   359618c79 old/busybox_INSMOD
28316 778 112   292067216 new/busybox_INSMOD

35228 962 112   363028dce old/busybox_LSMOD
 5011 706  405757167d new/busybox_LSMOD

34830 946 112   358888c30 old/busybox_MODPROBE
34795 946 112   358538c0d new/busybox_MODPROBE

34718 946 112   357768bc0 old/busybox_RMMOD
 7502 714 10483202080 new/busybox_RMMOD
From eac55bdd69d9e8a73bc886de6d64fa0fb5ba5b53 Mon Sep 17 00:00:00 2001
From: Explorer09 
Date: Mon, 9 Jan 2017 15:04:43 +0800
Subject: [RFC] modprobe-small: optimizations for single applet build

(I'm requesting for a review first because I fear such an aggressive
change could lead to bugs. While I observe the sizes have reduced, I
haven't test the functionality of each applet after that. So please
test before merging.)

Aggressively cut off unneeded code when the relevant applets are not
built.

Correct dependencies of FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE and
FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED.

Don't bother with the '-r' option check if only rmmod is built (assume
true then), or when neither rmmod or mobprobe is built (assume false
then).

Size comparison before and after the change (single applet
configuration):

 textdata bss dec hex filename
34778 946 112   358368bfc old/busybox_DEPMOD
34151 946 112   352098989 new/busybox_DEPMOD

34903 946 112   359618c79 old/busybox_INSMOD
28316 778 112   292067216 new/busybox_INSMOD

35228 962 112   363028dce old/busybox_LSMOD
 5011 706  405757167d new/busybox_LSMOD

34830 946 112   358888c30 old/busybox_MODPROBE
34795 946 112   358538c0d new/busybox_MODPROBE

34718 946 112   357768bc0 old/busybox_RMMOD
 7502 714 104    8320    2080 new/busybox_RMMOD

Signed-off-by: Kang-Che Sung 
---
 modutils/modprobe-small.c | 72 +--
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index 0fc9ea454..bc454e47a 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -13,15 +13,14 @@
 //config:config FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE
 //config:	bool "Accept module options on modprobe command line"
 //config:	default y
-//config:	depends on MODPROBE_SMALL
-//config:	select PLATFORM_LINUX
+//config:	depends on MODPROBE_SMALL && (INSMOD || MODPROBE)
 //config:	help
 //config:	  Allow insmod and modprobe take module options from command line.
 //config:
 //config:config FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED
 //config:	bool "Skip loading of already loaded modules"
 //config:	default y
-//config:	depends on MODPROBE_SMALL
+//config:	depends on MODPROBE_SMALL && (DEPMOD || INSMOD || MODPROBE)
 //config:	help
 //config:	  Check if the module is already loaded.
 
@@ -59,6 +58,14 @@
 
 #define DEPFILE_BB CONFIG_DEFAULT_DEPMOD_FILE".bb"
 
+#define ONLY_APPLET (ENABLE_MODPROBE + ENABLE_DEPMOD + ENABLE_INSMOD \
+  + ENABLE_LSMOD + ENABLE_RMMOD <= 1)
+#define is_modprobe (ENABLE_MODPROBE && (ONLY_APPLET || applet_name[0] == 'm'))
+#define is_depmod   (ENABLE_DEPMOD   && (ONLY_APPLET || applet_name[0] == 'd'))
+#define is_insmod   (ENABLE_INSMOD   && (ONLY_APPLET || applet_name[0] == 'i'))
+#define is_lsmod(ENABLE_LSMOD&& (ONLY_APPLET || applet_name[0] == 'l'))
+#define is_rmmod(ENABLE_RMMOD&& (ONLY_APPLET || applet_name[0] == 'r'))
+
 enum {
 	OPT_q = (1 << 0), /* be quiet */
 	OPT_r = (1 << 1), /* module removal instead of loading */
@@ -361,6 +368,8 @@ static FAST_FUNC int fileAction(const char *pathname,
 {
 	int cur;
 	const char *fname;
+	bool is_remove = ENABLE_RMMOD && ONLY_APPLET || (ENABLE_RMMOD
+		 || ENABLE_MODPROBE) && (option_mask32 

Re: [PATCH] Allow 'gzip -d' and 'bzip2 -d' without gunzip or bunzip2

[Kang-Che Sung]

On Mon, Jan 9, 2017 at 4:06 PM, Denys Vlasenko  wrote:
>
> FEATURE_BZIP2_DECOMPRESS has no type and is rejected by config system.
>
> #if FEATURE_BZIP2_DECOMPRESS is missing "ENABLE_".

Acknowledged the errors. Sorry then. Forgot to test that after editing
the code :(
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [RFC] modprobe-small: optimizations for single applet build

[Kang-Che Sung]

On Mon, Jan 9, 2017 at 4:49 PM, Denys Vlasenko  wrote:
> CONFIG_MODPROBE_SMALL=y
> CONFIG_DEPMOD=y
> # CONFIG_INSMOD is not set
> CONFIG_LSMOD=y
> # CONFIG_FEATURE_LSMOD_PRETTY_2_6_OUTPUT is not set
> CONFIG_MODINFO=y
> # CONFIG_MODPROBE is not set
> # CONFIG_FEATURE_MODPROBE_BLACKLIST is not set
> # CONFIG_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE is not set
> CONFIG_FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED=y
> # CONFIG_RMMOD is not set
>
> error: unused variable 'exitcode'
>
> modutils/modprobe-small.c: In function 'modprobe_main':
> modutils/modprobe-small.c:1060: error: control reaches end of non-void 
> function
>
Acknowledged. But I thought it was only a warning...
(The make_single_applets.sh didn't show compiler's or make's stderr to the
screen, and so I didn't notice. By the way, would make_single_applets.sh try to
do this trick? https://stackoverflow.com/questions/2871233/)

And great job on making another optimization with depmod and lsmod two-applet
configuration.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 1/2] Need not build kill.c when ash's job control is off.

[Kang-Che Sung]

ash's 'kill' builtin depends on the job control config option.

Signed-off-by: Kang-Che Sung 
---
 procps/Kbuild.src | 5 +
 procps/kill.c | 3 +--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/procps/Kbuild.src b/procps/Kbuild.src
index 82f37f0df..dedef8881 100644
--- a/procps/Kbuild.src
+++ b/procps/Kbuild.src
@@ -8,8 +8,5 @@ lib-y:=

 INSERT

-lib-$(CONFIG_ASH) += kill.o  # used for built-in kill by ash
-lib-$(CONFIG_SH_IS_ASH) += kill.o  # used for built-in kill by ash
-lib-$(CONFIG_BASH_IS_ASH) += kill.o  # used for built-in kill by ash
-
+lib-$(CONFIG_ASH_JOB_CONTROL) += kill.o  # used for built-in kill by ash
 lib-$(CONFIG_HUSH_KILL) += kill.o  # used for built-in kill by hush
diff --git a/procps/kill.c b/procps/kill.c
index 579c8e53c..7c0822542 100644
--- a/procps/kill.c
+++ b/procps/kill.c
@@ -285,8 +285,7 @@ int kill_main(int argc UNUSED_PARAM, char **argv)

  /* Looks like they want to do a kill. Do that */
  while (arg) {
-#if ENABLE_ASH || ENABLE_SH_IS_ASH || ENABLE_BASH_IS_ASH \
- || ENABLE_HUSH_KILL
+#if ENABLE_ASH_JOB_CONTROL || ENABLE_HUSH_KILL
  /*
  * We need to support shell's "hack formats" of
  * " -PRGP_ID" (yes, with a leading space)
-- 
2.11.0
From fc4e247b698fbd656d31e340c649b2cbfcb13f77 Mon Sep 17 00:00:00 2001
From: Kang-Che Sung 
Date: Mon, 9 Jan 2017 22:41:49 +0800
Subject: [PATCH 1/2] Need not build kill.c when ash's job control is off.

ash's 'kill' builtin depends on the job control config option.

Signed-off-by: Kang-Che Sung 
---
 procps/Kbuild.src | 5 +
 procps/kill.c | 3 +--
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/procps/Kbuild.src b/procps/Kbuild.src
index 82f37f0df..dedef8881 100644
--- a/procps/Kbuild.src
+++ b/procps/Kbuild.src
@@ -8,8 +8,5 @@ lib-y:=
 
 INSERT
 
-lib-$(CONFIG_ASH) += kill.o  # used for built-in kill by ash
-lib-$(CONFIG_SH_IS_ASH) += kill.o  # used for built-in kill by ash
-lib-$(CONFIG_BASH_IS_ASH) += kill.o  # used for built-in kill by ash
-
+lib-$(CONFIG_ASH_JOB_CONTROL) += kill.o  # used for built-in kill by ash
 lib-$(CONFIG_HUSH_KILL) += kill.o  # used for built-in kill by hush
diff --git a/procps/kill.c b/procps/kill.c
index 579c8e53c..7c0822542 100644
--- a/procps/kill.c
+++ b/procps/kill.c
@@ -285,8 +285,7 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
 
 	/* Looks like they want to do a kill. Do that */
 	while (arg) {
-#if ENABLE_ASH || ENABLE_SH_IS_ASH || ENABLE_BASH_IS_ASH \
- || ENABLE_HUSH_KILL
+#if ENABLE_ASH_JOB_CONTROL || ENABLE_HUSH_KILL
 		/*
 		 * We need to support shell's "hack formats" of
 		 * " -PRGP_ID" (yes, with a leading space)
-- 
2.11.0

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

[PATCH 2/2] Document ash and hush config options in more detail.

[Kang-Che Sung]

Which of the bash-compatible extensions are supported in ash and hush
are now listed.

Also be precise about which of the builtins will be enabled in each
option.

(By the way, I didn't yet have an idea about what ash's "monitor"
(set -m) option will do when job control is disabled at build time.)

Signed-off-by: Kang-Che Sung 
---
 shell/ash.c  | 30 +-
 shell/hush.c | 15 ++-
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/shell/ash.c b/shell/ash.c
index 9c46a93e0..272682d78 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -72,7 +72,18 @@
 //config: default y
 //config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
-//config:  Enable bash-compatible extensions.
+//config:  Enable bash-compatible extensions, which currently include:
+//config:  - 'source' builtin
+//config:  - '[[' builtin (if 'test' builtin is also enabled)
+//config:  - 'function' keyword
+//config:  - pipefail option ('set -o pipefail')
+//config:  - $'...' expansion
+//config:  - ${var:position:length} expansion
+//config:  - ${var/pattern/replacement} expansion
+//config:  - ${var//pattern/replacement} expansion
+//config:  - $HOSTNAME variable
+//config:  - $SHLVL variable
+//config:  - '&>' redirection ('&>file' equivalent to '>file 2>&1')
 //config:
 //config:config ASH_IDLE_TIMEOUT
 //config: bool "Idle timeout variable"
@@ -87,48 +98,49 @@
 //config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
 //config:  Enable job control in the ash shell.
+//config:  Enable these builtins: bg, fg, jobs, kill
 //config:
 //config:config ASH_ALIAS
 //config: bool "Alias support"
 //config: default y
 //config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
-//config:  Enable alias support in the ash shell.
+//config:  Enable support for 'alias' and 'unalias' builtins in ash.
 //config:
 //config:config ASH_GETOPTS
 //config: bool "Builtin getopt to parse positional parameters"
 //config: default y
 //config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
-//config:  Enable support for getopts builtin in ash.
+//config:  Enable support for 'getopts' builtin in ash.
 //config:
 //config:config ASH_BUILTIN_ECHO
 //config: bool "Builtin version of 'echo'"
 //config: default y
 //config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
-//config:  Enable support for echo builtin in ash.
+//config:  Enable support for 'echo' builtin in ash.
 //config:
 //config:config ASH_BUILTIN_PRINTF
 //config: bool "Builtin version of 'printf'"
 //config: default y
 //config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
-//config:  Enable support for printf builtin in ash.
+//config:  Enable support for 'printf' builtin in ash.
 //config:
 //config:config ASH_BUILTIN_TEST
-//config: bool "Builtin version of 'test'"
+//config: bool "Builtin version of 'test' and '['"
 //config: default y
 //config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
-//config:  Enable support for test builtin in ash.
+//config:  Enable support for 'test' and '[' builtins in ash.
 //config:
 //config:config ASH_HELP
 //config: bool "help builtin"
 //config: default y
 //config: depends on ASH || SH_IS_ASH || BASH_IS_ASH
 //config: help
-//config:  Enable help builtin in ash.
+//config:  Enable 'help' builtin in ash.
 //config:
 //config:config ASH_CMDCMD
 //config: bool "'command' command to override shell builtins"
@@ -3943,7 +3955,7 @@ fg_bgcmd(int argc UNUSED_PARAM, char **argv)
  } while (*argv && *++argv);
  return retval;
 }
-#endif
+#endif /* JOBS */

 static int
 sprint_status48(char *s, int status, int sigonly)
diff --git a/shell/hush.c b/shell/hush.c
index 5c5715b3f..fe9444181 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -100,7 +100,17 @@
 //config: default y
 //config: depends on HUSH || SH_IS_HUSH || BASH_IS_HUSH
 //config: help
-//config:  Enable bash-compatible extensions.
+//config:  Enable bash-compatible extensions, which currently include:
+//config:  - 'source' builtin
+//config:  - '[[' builtin ('test' and '[' builtin are always enabled
+//config:regardless)
+//config:  - ${var:position:length} expansion (if math support is also
+//config:enabled)
+//config:  - ${var/pattern/replacement} expansion
+//config:  - ${var//pattern/replacement} expansion
+//config:  - $HOSTNAME variable
+//config:  Brace expansion is available as a separate config option
+//config:  (HUSH_BRACE_EXPANSION).
 //config:
 //config:config HUSH_BRACE_EXPANSION
 //config: bool "Brace expansion"
@@ -136,6 +146,9 

Re: [PATCH 1/2] Need not build kill.c when ash's job control is off.

[Kang-Che Sung]

On Mon, Jan 9, 2017 at 11:03 PM, Jody Bruchon  wrote:
> On 2017-01-09 9:59 AM, Kang-Che Sung wrote:
>>
>> ash's 'kill' builtin depends on the job control config option.
>>
> What about external 'kill' command /bin/kill? It should be built if
> requested even if ash job control is disabled.

The "external" kill command for busybox is unaffected by this patch.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] kill: optimizations for single-applet build

[Kang-Che Sung]

* Fix a bug with a configuration in which the shell's kill builtin
  would be mistreated as a killall command (i.e. '-q' works, and
  'kill process_name' succeeds when it shouldn't):

CONFIG_ASH_JOB_CONTROL=y
CONFIG_HUSH_KILL=y
# CONFIG_KILL is not set
CONFIG_KILLALL=y
# CONFIG_KILLALL5 is not set

* Optimize out unneeded code when the relevant applets are not
  selected.

* Move kbuild lines about shells' kill builtins from Kbuild.src to
  kill.c, to accompany the new HAVE_SH_KILL macro. I hope this would
  make maintanence a little bit easier.
From 7296087fc1e2b729477ce5c3cb1a2b254370510e Mon Sep 17 00:00:00 2001
From: Kang-Che Sung 
Date: Tue, 10 Jan 2017 00:26:31 +0800
Subject: [PATCH] kill: optimizations for single-applet build

* Fix a bug with a configuration in which the shell's kill builtin
  would be mistreated as a killall command (i.e. '-q' works, and
  'kill process_name' succeeds when it shouldn't):

CONFIG_ASH_JOB_CONTROL=y
CONFIG_HUSH_KILL=y
# CONFIG_KILL is not set
CONFIG_KILLALL=y
# CONFIG_KILLALL5 is not set

* Optimize out unneeded code when the relevant applets are not
  selected.

* Move kbuild lines about shells' kill builtins from Kbuild.src to
  kill.c, to accompany the new HAVE_SH_KILL macro. I hope this would
  make maintanence a little bit easier.

Signed-off-by: Kang-Che Sung 
---
 procps/Kbuild.src |  2 --
 procps/kill.c | 45 ++---
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/procps/Kbuild.src b/procps/Kbuild.src
index dedef8881..a2cc53ca0 100644
--- a/procps/Kbuild.src
+++ b/procps/Kbuild.src
@@ -8,5 +8,3 @@ lib-y:=
 
 INSERT
 
-lib-$(CONFIG_ASH_JOB_CONTROL) += kill.o  # used for built-in kill by ash
-lib-$(CONFIG_HUSH_KILL) += kill.o  # used for built-in kill by hush
diff --git a/procps/kill.c b/procps/kill.c
index 7c0822542..bcd424717 100644
--- a/procps/kill.c
+++ b/procps/kill.c
@@ -92,28 +92,30 @@
  * This is needed to avoid collision with kill -9 ... syntax
  */
 
+//kbuild:lib-$(CONFIG_ASH_JOB_CONTROL) += kill.o  # used for built-in kill by ash
+//kbuild:lib-$(CONFIG_HUSH_KILL) += kill.o  # used for built-in kill by hush
+#define HAVE_SH_KILL (ENABLE_ASH_JOB_CONTROL || ENABLE_HUSH_KILL)
+
+#define KILL_APPLET_CNT ((ENABLE_KILL || HAVE_SH_KILL) + ENABLE_KILLALL + ENABLE_KILLALL5)
+#define ONLY_APPLET (KILL_APPLET_CNT == 1)
+
 int kill_main(int argc UNUSED_PARAM, char **argv)
 {
 	char *arg;
 	pid_t pid;
 	int signo = SIGTERM, errors = 0, quiet = 0;
-#if ENABLE_KILL && !ENABLE_KILLALL && !ENABLE_KILLALL5
-# define killall  0
-# define killall5 0
-#elif !ENABLE_KILL && ENABLE_KILLALL && !ENABLE_KILLALL5
-# define killall  1
-# define killall5 0
-#elif !ENABLE_KILL && !ENABLE_KILLALL && ENABLE_KILLALL5
-# define killall  0
-# define killall5 1
+
+#if ONLY_APPLET
+# define is_killall  ENABLE_KILLALL
+# define is_killall5 ENABLE_KILLALL5
 #else
 /* How to determine who we are? find 3rd char from the end:
  * kill, killall, killall5
  *  ^i   ^a^l  - it's unique
  * (checking from the start is complicated by /bin/kill... case) */
 	const char char3 = argv[0][strlen(argv[0]) - 3];
-# define killall  (ENABLE_KILLALL && char3 == 'a')
-# define killall5 (ENABLE_KILLALL5 && char3 == 'l')
+# define is_killall  (ENABLE_KILLALL  && char3 == 'a')
+# define is_killall5 (ENABLE_KILLALL5 && char3 == 'l')
 #endif
 
 	/* Parse any options */
@@ -162,7 +164,7 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
 	}
 
 	/* The -q quiet option */
-	if (killall && arg[1] == 'q' && arg[2] == '\0') {
+	if (is_killall && arg[1] == 'q' && arg[2] == '\0') {
 		quiet = 1;
 		arg = *++argv;
 		if (!arg)
@@ -174,7 +176,7 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
 	arg++; /* skip '-' */
 
 	/* -o PID? (if present, it always is at the end of command line) */
-	if (killall5 && arg[0] == 'o')
+	if (is_killall5 && arg[0] == 'o')
 		goto do_it_now;
 
 	if (argv[1] && arg[0] == 's' && arg[1] == '\0') { /* -s SIG? */
@@ -190,7 +192,7 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
  do_it_now:
 	pid = getpid();
 
-	if (killall5) {
+	if (is_killall5) {
 		pid_t sid;
 		procps_status_t* p = NULL;
 		/* compat: exitcode 2 is "no one was signaled" */
@@ -239,7 +241,8 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
 			}
 			kill(p->pid, signo);
 			ret = 0;
- dont_kill: ;
+ dont_kill:
+			continue;
 		}
  resume:
 		/* And let them continue */
@@ -248,13 +251,14 @@ int kill_main(int argc UNUSED_PARAM, char **argv)
 		return ret;
 	}
 
+#if ENABLE_KILL || HAVE_SH_KILL || ENABLE_KILLALL
 	/* Pid or name is require

Re: [PATCH] dpkg & dpkg_deb: don't depend on FEATURE_SEAMLESS_GZ

[Kang-Che Sung]

On Tue, Jan 10, 2017 at 12:43 AM, Denys Vlasenko
 wrote:
> On Sat, Jan 7, 2017 at 2:48 AM, Kang-Che Sung  wrote:
>>>> My understanding is that .deb files usually use .gz compression,
>>>> and building dpkg without support for .gz results in a useless tool:
>>>> there are no .deb files which it can process.
>>>
>>> Debian packages support gzip or xz for the control.tar file and a variety of
>>> common compression formats for the data.tar file.
>>
>> Yes. And my point is that there's no need to force a gz choice for users.
>> gz could be deprecated. And sometimes a custom distribution may decide
>> not to gz-compress its .deb packages at all.
>>
>> I think it will be better to just *recommend* the gz feature instead. Mention
>> in the help text:
>>
>> Note that most .deb packages compress their metadata
>> in gz (control.tar.gz), so you are likely to also enable the
>> "understand .gz data" feature above (FEATURE_SEAMLESS_GZ).
>
> I still think the downsides (many people inadvertently building non-functional
> dpkg and complaining) are bigger than win for a rare case when someone gets
> .gz support he doesn't need.

Sigh. If only busybox's kconfig would support the "imply" clause (which a recent
addition in Linux kernel's kconfig though)...

For now, I wonder if it is a good idea to implement a warning or workaround such
as this:

config FEATURE_DPKG_WITH_GZ
  bool "dpkg or dpkg-deb with gzip decompression"
  default y
  depends on DPKG || DPKG_DEB
  select FEATURE_SEAMLESS_GZ
  help
Most .deb packages contain gzip-compressed metadata
(control.tar.gz) and so
will require gzip decompression.

Just say Y unless you know you are never working with most .deb packages
on the Internet (i.e. you are working with custom .deb packages only).

This option alone serves as a warning only and does not add any code.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Please revert "ash: make dot command search current directory first, as bash does."

[Kang-Che Sung]

Please revert commit 8ad78e1ec7b2e873953f9f476fb63b5893526c39
"ash: make dot command search current directory first, as bash does."

This dot command behavior neither follows bash's behavior nor POSIX's.

Bash behavior is:
Dot command search on PATH first, *then* search the current directory.
And bash allows this behavior to turn off via POSIX compatibility mode
('set -o posix') [1]

POSIX behavior is:
Search for PATH only. Don't bother with current directory unless . is part
of PATH. [2]

BusyBox ash's behavior becomes neither of the two, so the change should be
reverted.

References:
[1] https://www.gnu.org/software/bash/manual/html_node/Bash-POSIX-Mode.html
"[With POSIX mode, t]he . and source builtins do not search the current
directory for the filename argument if it is not found by searching PATH."
[2] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#dot
(It didn't mention about current directory, only PATH.)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 1/2] hush: add [[ to the builtins list

[Kang-Che Sung]

Otherwise the [[ EXPR ]] construct will not run.
"hush: can't execute '[[': No such file or directory"

It seems like this thing has not been working from the beginning :(
(commit 9d617c44d2b1135d14b7dafd01a1d3992293f4d9)

Signed-off-by: Kang-Che Sung 
---
 shell/hush.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/shell/hush.c b/shell/hush.c
index ecef099ac..68b838378 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -1073,6 +1073,9 @@ static const struct built_in_command bltins1[] = {
 static const struct built_in_command bltins2[] = {
 #if ENABLE_HUSH_TEST
  BLTIN("[", builtin_test, NULL),
+# if ENABLE_HUSH_BASH_COMPAT
+ BLTIN("[["   , builtin_test, NULL),
+# endif
 #endif
 #if ENABLE_HUSH_ECHO
  BLTIN("echo" , builtin_echo, NULL),
-- 
2.11.0
From d041fce007fdf0c4072de213d8fcc1d621469469 Mon Sep 17 00:00:00 2001
From: Kang-Che Sung 
Date: Wed, 11 Jan 2017 02:37:35 +0800
Subject: [PATCH 1/2] hush: add [[ to the builtins list

Otherwise the [[ EXPR ]] construct will not run.
"hush: can't execute '[[': No such file or directory"

It seems like this thing has not been working from the beginning :(
(commit 9d617c44d2b1135d14b7dafd01a1d3992293f4d9)

Signed-off-by: Kang-Che Sung 
---
 shell/hush.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/shell/hush.c b/shell/hush.c
index ecef099ac..68b838378 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -1073,6 +1073,9 @@ static const struct built_in_command bltins1[] = {
 static const struct built_in_command bltins2[] = {
 #if ENABLE_HUSH_TEST
 	BLTIN("[", builtin_test, NULL),
+# if ENABLE_HUSH_BASH_COMPAT
+	BLTIN("[["   , builtin_test, NULL),
+# endif
 #endif
 #if ENABLE_HUSH_ECHO
 	BLTIN("echo" , builtin_echo, NULL),
-- 
2.11.0

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

[PATCH 2/2] Split bash compatible extensions into separate options.

[Kang-Che Sung]

(Note: this is a large patch. Patch included as mail attachment)

All of the bash compatibility options will now depend on
CONFIG_*_BASH_COMPAT, and CONFIG_*_BASH_COMPAT become an option that
alone doesn't add any code.

Splitting these options allows more flexibility in configuration, and
makes it self-documenting about what bash-compatible features we have.

Signed-off-by: Kang-Che Sung 
---
 coreutils/test.c |   8 ++-
 shell/ash.c  | 216 +--
 shell/hush.c | 133 ++
 3 files changed, 239 insertions(+), 118 deletions(-)
From 655b1f7dd97c83c79502a0635fe9d195de47beaa Mon Sep 17 00:00:00 2001
From: Kang-Che Sung 
Date: Wed, 11 Jan 2017 03:02:24 +0800
Subject: [PATCH 2/2] Split bash compatible extensions into separate options.

All of the bash compatibility options will now depend on
CONFIG_*_BASH_COMPAT, and CONFIG_*_BASH_COMPAT become an option that
alone doesn't add any code.

Splitting these options allows more flexibility in configuration, and
makes it self-documenting about what bash-compatible features we have.

Signed-off-by: Kang-Che Sung 
---
 coreutils/test.c |   8 ++-
 shell/ash.c  | 216 +--
 shell/hush.c | 133 ++
 3 files changed, 239 insertions(+), 118 deletions(-)

diff --git a/coreutils/test.c b/coreutils/test.c
index edc625f57..704255630 100644
--- a/coreutils/test.c
+++ b/coreutils/test.c
@@ -42,7 +42,7 @@
 //config:config FEATURE_TEST_64
 //config:	bool "Extend test to 64 bit"
 //config:	default y
-//config:	depends on TEST || TEST1 || TEST2 || ASH_TEST || HUSH_TEST
+//config:	depends on TEST || TEST1 || TEST2 || ASH_TEST || ASH_TEST2 || HUSH_TEST || HUSH_TEST2
 //config:	help
 //config:	  Enable 64-bit support in test.
 
@@ -54,8 +54,10 @@
 //kbuild:lib-$(CONFIG_TEST1) += test.o test_ptr_hack.o
 //kbuild:lib-$(CONFIG_TEST2) += test.o test_ptr_hack.o
 
-//kbuild:lib-$(CONFIG_ASH_TEST)  += test.o test_ptr_hack.o
-//kbuild:lib-$(CONFIG_HUSH_TEST) += test.o test_ptr_hack.o
+//kbuild:lib-$(CONFIG_ASH_TEST)   += test.o test_ptr_hack.o
+//kbuild:lib-$(CONFIG_ASH_TEST2)  += test.o test_ptr_hack.o
+//kbuild:lib-$(CONFIG_HUSH_TEST)  += test.o test_ptr_hack.o
+//kbuild:lib-$(CONFIG_HUSH_TEST2) += test.o test_ptr_hack.o
 
 /* "test --help" is special-cased to ignore --help */
 //usage:#define test_trivial_usage NOUSAGE_STR
diff --git a/shell/ash.c b/shell/ash.c
index 866c7de05..d5f6b7d04 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -47,11 +47,6 @@
 //config:	  Note that as of now (2017-01), uclibc and musl glob() both have bugs
 //config:	  which would break ash if you select N here.
 //config:
-//config:config ASH_BASH_COMPAT
-//config:	bool "bash-compatible extensions"
-//config:	default y
-//config:	depends on ASH || SH_IS_ASH || BASH_IS_ASH
-//config:
 //config:config ASH_JOB_CONTROL
 //config:	bool "Job control"
 //config:	default y
@@ -133,6 +128,76 @@
 //config:	  you to run the specified command or builtin,
 //config:	  even when there is a function with the same name.
 //config:
+//config:config ASH_BASH_COMPAT
+//config:	bool "bash-compatible extensions"
+//config:	default y
+//config:	depends on ASH || SH_IS_ASH || BASH_IS_ASH
+//config:
+//config:config ASH_DOLLAR_SQUOTE
+//config:	bool "Dollar single quote $'...'"
+//config:	default y
+//config:	depends on ASH_BASH_COMPAT
+//config:
+//config:config ASH_SUBSTR_EXPANSION
+//config:	bool "Substring expansion ${var:position:length}"
+//config:	default y
+//config:	depends on ASH_BASH_COMPAT
+//config:
+//config:config ASH_PATTERN_SUBST
+//config:	bool "Pattern substitution ${var/pattern/replacement}"
+//config:	default y
+//config:	depends on ASH_BASH_COMPAT
+//config:
+//config:config ASH_REDIR_OUTPUT2
+//config:	bool "Redirection operator &>file (equivalent to '>file 2>&1')"
+//config:	default y
+//config:	depends on ASH_BASH_COMPAT
+//config:
+//config:config ASH_FUNCTION2
+//config:	bool "function keyword"
+//config:	default y
+//config:	depends on ASH_BASH_COMPAT
+//config:	help
+//config:	  Enable the 'function' keyword to define a function in
+//config:	  addition to the standard 'funcname() { commands; }' syntax.
+//config:	  'function' keyword is bash-specific.
+//config:
+//config:config ASH_SOURCE
+//config:	bool "source builtin"
+//config:	default y
+//config:	depends on ASH_BASH_COMPAT
+//config:	help
+//config:	  Enable source builtin, which is equivalent to . (dot) builtin.
+//config:	  The availabilty of . (dot) builtin is unaffected by this option.
+//config:
+//config:config ASH_TEST2
+//config:	bool "test construct [[ EXPR ]]"
+//config:	default y
+//config:	depends on ASH_BASH_COMPAT
+//config:	help
+//config:	  The test construct [[ EXPR ]] sup

Re: [PATCH 2/2] Split bash compatible extensions into separate options.

[Kang-Che Sung]

On Wed, Jan 11, 2017 at 3:17 AM, Kang-Che Sung  wrote:
> (Note: this is a large patch. Patch included as mail attachment)
>
> All of the bash compatibility options will now depend on
> CONFIG_*_BASH_COMPAT, and CONFIG_*_BASH_COMPAT become an option that
> alone doesn't add any code.
>
> Splitting these options allows more flexibility in configuration, and
> makes it self-documenting about what bash-compatible features we have.
>
> Signed-off-by: Kang-Che Sung 
> ---
>  coreutils/test.c |   8 ++-
>  shell/ash.c  | 216 
> +--
>  shell/hush.c | 133 ++
>  3 files changed, 239 insertions(+), 118 deletions(-)

Oops. I missed the ASH_BASH_COMPAT help text. It should be the same as
hush's one.

//config: help
//config:  Saying N to this will skip all options about bash-compatible
//config:  extensions. This option alone does not add any code.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH 2/2] Split bash compatible extensions into separate options.

[Kang-Che Sung]

On Wed, Jan 11, 2017 at 4:09 PM, Denys Vlasenko
 wrote:
> On Tue, Jan 10, 2017 at 8:17 PM, Kang-Che Sung  wrote:
>> All of the bash compatibility options will now depend on
>> CONFIG_*_BASH_COMPAT, and CONFIG_*_BASH_COMPAT become an option that
>> alone doesn't add any code.
>>
>> Splitting these options allows more flexibility in configuration
>
> Configuration shouldn't strive to be maximally flexible.
>
> It floods the user with way too many options.
> For example, now LS has eight suboptions.
> Imagine this instead:
>
> [*] ls
> [*]   Enable -1One column output
> [*]   Enable -aInclude entries which start with .
> [*]   Enable -ALike -a, but exclude . and ..
> [*]   Enable -CList by columns
> [*]   Enable -xList by lines
> [*]   Enable -dList directory entries instead of contents
> [*]   Enable -LFollow symlinks
> [*]   Enable -HFollow symlinks on command line
> [*]   Enable -RRecurse
> [*]   Enable -pAppend / to dir entries
> [*]   Enable -FAppend indicator (one of */=@|) to entries
> [*]   Enable -lLong listing format
> [*] Enable -c  sort by ctime
> [*] Enable -t  sort by mtime
> [*] Enable -u  sort by atime
> [*]   Enable -iList inode numbers
> [*]   Enable -nList numeric UIDs and GIDs instead of names
> [*]   Enable -sList allocated blocks
> [*]   Enable -eList full date and time
> [*]   Enable -hList sizes in human readable format (1K 243M 2G)
> [*]   Enable -rSort in reverse order
> [*]   Enable -SSort by size
> [*]   Enable -XSort by extension
> [*]   Enable -vSort by version
> [*]   Enable -w N  Assume the terminal is N columns wide
> [*]   Enable --color
> [*] Produce colored ls output by default
>
> For every applet and every option. That would be ridiculous.
>
> Ridiculous we can live with, but there is a bigger problem too.
> More options means more #ifdefs and more weird interactions
> between different options, making code less maintainable.
> Because of this reason, some options (e.g. in top) were even
> _removed_ (permanently enabled) because code was too tangled.
>
> therefore options should be introduced if there is a reasonable
> chance user actually needs to control them separately,
> and if they control a nontrivial amount of code (i.e. not 20 bytes).
>
> If user needs bash compat, it means he is not running on an extremely
> constrained machine (otherwise he can well spend a bit of time and rewrite
> all his scripts to be POSIX-compatible). Since the machine is not
> too constrained, it's okay to just enable all of them and not worry
> about having a few more kbytes of code for a bashism he doesn't need.

I don't buy your argument, unfortunately.

My points:

1. I can read if any of the shell's feature is too trivial that it'll
be not worth
implementing or making a configure option at all. The bash-compatible
'source' command behavior, that I suggested you to revert, is one of the
example. (busybox ash implemented that wrong, and I know user can
easily have 'source' search for current directory by adding '.' to PATH,
meaning that it'll be not worthy to implement it, even though it could
implement right.)

2. The bash-compat extensions are already scattered around various
places in the source code. And each of the
#if ENABLE_ASH_BASH_COMPAT ... #endif blocks only cover one or
two features that can break up easily. They share little common code.

3. If people don't like too many Kconfig options, they don't have to.
Nothing stops you from making them in-C macros instead:

#if ENABLE_ASH_BASH_COMPAT
# define ENABLE_ASH_DOLLAR_SQUOTE/* $'...' */
# define ENABLE_ASH_SUBSTR_EXPANSION /* ${var:pos:len} */
# define ENABLE_ASH_PATTERN_SUBST/* ${var/pattern/repl} */
# define ENABLE_ASH_REDIR_OUTPUT2/* &>file ; >&file */
# define ENABLE_ASH_FUNCTION2/* function keyword */
# define ENABLE_ASH_SOURCE   /* source keyword */
# define ENABLE_ASH_TEST2/* [[ EXPR ]] */
# define ENABLE_ASH_OPT_PIPEFAIL /* -o pipefail */
# define ENABLE_ASH_HOSTNAME_VAR /* $HOSTNAME */
# define ENABLE_ASH_SHLVL_VAR/* $SHLVL */
#endif

#if ENABLE_HUSH_BASH_COMPAT
# define ENABLE_HUSH_BRACE_EXPANSION
# define ENABLE_HUSH_SUBSTR_EXPANSION
# define ENABLE_HUSH_PATTERN_SUBST
# define ENABLE_HUSH_SOURCE
# define ENABLE_HUSH_TEST2
# define ENABLE_HUSH_HOSTNAME_VAR
#endif

4. If we want to argue with "trivial", then I think only a few of the above fits
('source', 'function', '$SHLVL', and '$HOSTNAME')
The others, such as substring expansion and pattern substitution, actually
add a couple of subroutines that I don't think just 20 bytes of code could
make it.

5. I would wonder why HUSH_BRACE_EXPANSION is separate from
HUSH_BASH_COMPAT in the first place...
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: bugs in a couple scripts

[Kang-Che Sung]

On Sun, Jan 22, 2017 at 6:56 AM, J  wrote:
> Hello mailing list,
>
> i found (the hard way) that scripts/gen_build_files.sh is making unsafe
> assumptions about the nature of /bin/sh; it appears to be presuming that
> it's bash or dash or something:
> --
>   GEN runit/Kbuild
> /usr/src/install/busybox-1.26.2/scripts/gen_build_files.sh[23]: local: not
> found [No such file or directory]
>   GEN runit/Config.in
>   HOSTCC  scripts/basic/fixdep
> /usr/src/install/busybox-1.26.2/scripts/gen_build_files.sh[23]: local: not
> found [No such file or directory]
> sed: can't read : No such file or directory
> sed: can't read : No such file or directory
>   GEN
> mv: cannot rename .tmp to : No such file or directory
> /usr/src/install/busybox-1.26.2/scripts/gen_build_files.sh[23]: local: not
> found [No such file or directory]
> sed: can't read : No such file or directory
>   HOSTCC  scripts/basic/docproc
>   HOSTCC  scripts/basic/split-include
> sed: can't read : No such file or directory
>   GEN
> mv: cannot rename .tmp to : No such file or directory
> /usr/src/install/busybox-1.26.2/scripts/gen_build_files.sh[23]: local: not
> found [No such file or directory]
> /usr/src/install/busybox-1.26.2/scripts/gen_build_files.sh[23]: local: not
> found [No such file or directory]
> --
>
> and so on. attached is a patch which fixes this script and thus allows
> busybox to build. while i was in scripts/ i also noticed that the "trylink"
> script is making the same wrong assumption, so i went ahead and patched it
> as well to be safe, and i've attached that also.
>

Your attached scripts.trylink.diff has bugs. It seems that you just found-and-
replaced the variables without verifying the script further.

It should give you "cc_r: command not found" and "lig_r: command not found".
I hope you know where I'm talking about.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Why preferring smallint to bool in some places?

[Kang-Che Sung]

Dear busybox developers,

May I ask some little code questions? I have been reading some of the
changes in busybox and I don't understand the use of type "smallint" in
this code

9a64c3337cc0a5e84e9ad457eeb1d475c311e9fc "ls: convert DISP_DIRNAME to
a bool variable"

ls.c

@@ -330,7 +326,7 @@ struct globals {
 # define G_show_color 0
 #endif
  smallint exit_code;
- unsigned all_fmt;
+ smallint show_dirname;
 #if ENABLE_FEATURE_LS_WIDTH
  unsigned terminal_width;
 # define G_terminal_width (G.terminal_width)

Here it seems that this "show_dirname" variable is supposed to hold a
boolean value, but why not declare it with the type bool? What's the
rationale behind preferring smallint?

Thank you.
Kang-Che Sung
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] add exec -a support (preliminary)

[Kang-Che Sung]

On Fri, Jan 27, 2017 at 7:56 AM, Patrick Pief  wrote:
> There were several times where I thought that having "exec -a" in busybox 
> would
> be neat, and while "exec -a" is not POSIX it is still supported in a lot of
> shells (see http://unix.stackexchange.com/q/250681/117599 ).
>
> So, here is now my first attempt at adding support for it, I basically made a
> copy of shellexec() called shellexeca() with support for a different target
> argv0 different than what is executed. I edited execcmd() to call the new
> function if the "-a" option gets used. I know that you might not like the
> fact that I duplicated that much code but keep in mind that this is my first
> attempt and I would first like to know:
>
> 1.  Your general feelings on this, e.g. if you are against this feature in
> general and are never going to accept a PATCH adding support for it then
> please let me know.
>
> 2.  Whether to hide this using the preprocessor flag and make it an official
> setting.
>
> 3.  Whether to make shellexeca() to be the new shellexec() and change all
> invocations of latter (I think this would make hiding it using the
> preprocessor harder).

I would personally prefer hiding your change in a preprocessor flag. That way
people who don't want that feature won't need to build it. I'm not
sure if this is
going to be a separate config option or just a BASH_ preprocessor symbol,
though. (Better ask Denys Vlasenko, the maintainer.)

And, don't duplicate another function if what you did is only extending a
functionality of that function. BusyBox is coded with size in mind. Duplicating
a function will mean wasting space. Besides, if you code within shellexec() we
can see a better diff on what you have extended.

Kang-Che Sung
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] Fix modutils config options dependency.

[Kang-Che Sung]

module.aliases and module.symbols files have no use in modprobe-small
implementation. So FEATURE_MODUTILS_ALIAS and FEATURE_MODUTILS_SYMBOLS
will depend on !MODPROBE_SMALL.

The try_to_mmap_module() function is not called in modprobe-small.c,
so I will let FEATURE_INSMOD_TRY_MMAP depend on !MODPROBE_SMALL for
now. I'm not sure whether the lack of try_to_mmap_module in
modprobe-small is intentional. If not, please reject my changes.

Signed-off-by: Kang-Che Sung 
---
 configs/TEST_nommu_defconfig  | 3 ---
 configs/TEST_rh9_defconfig| 3 ---
 configs/android2_defconfig| 3 ---
 configs/android_502_defconfig | 3 ---
 configs/android_defconfig | 3 ---
 configs/android_ndk_defconfig | 3 ---
 modutils/Config.src   | 6 +++---
 7 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/configs/TEST_nommu_defconfig b/configs/TEST_nommu_defconfig
index b7d9a35c2..7fbbbecc7 100644
--- a/configs/TEST_nommu_defconfig
+++ b/configs/TEST_nommu_defconfig
@@ -452,15 +452,12 @@ CONFIG_FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED=y
 # Options common to multiple modutils
 #
 # CONFIG_FEATURE_2_4_MODULES is not set
-CONFIG_FEATURE_INSMOD_TRY_MMAP=y
 # CONFIG_FEATURE_INSMOD_VERSION_CHECKING is not set
 # CONFIG_FEATURE_INSMOD_KSYMOOPS_SYMBOLS is not set
 # CONFIG_FEATURE_INSMOD_LOADINKMEM is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP_FULL is not set
 # CONFIG_FEATURE_CHECK_TAINTED_MODULE is not set
-# CONFIG_FEATURE_MODUTILS_ALIAS is not set
-# CONFIG_FEATURE_MODUTILS_SYMBOLS is not set
 CONFIG_DEFAULT_MODULES_DIR="/lib/modules"
 CONFIG_DEFAULT_DEPMOD_FILE="modules.dep"

diff --git a/configs/TEST_rh9_defconfig b/configs/TEST_rh9_defconfig
index 99deb67fe..34d8e31e2 100644
--- a/configs/TEST_rh9_defconfig
+++ b/configs/TEST_rh9_defconfig
@@ -467,15 +467,12 @@ CONFIG_FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED=y
 # Options common to multiple modutils
 #
 # CONFIG_FEATURE_2_4_MODULES is not set
-# CONFIG_FEATURE_INSMOD_TRY_MMAP is not set
 # CONFIG_FEATURE_INSMOD_VERSION_CHECKING is not set
 # CONFIG_FEATURE_INSMOD_KSYMOOPS_SYMBOLS is not set
 # CONFIG_FEATURE_INSMOD_LOADINKMEM is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP_FULL is not set
 # CONFIG_FEATURE_CHECK_TAINTED_MODULE is not set
-# CONFIG_FEATURE_MODUTILS_ALIAS is not set
-# CONFIG_FEATURE_MODUTILS_SYMBOLS is not set
 CONFIG_DEFAULT_MODULES_DIR="/lib/modules"
 CONFIG_DEFAULT_DEPMOD_FILE="modules.dep"

diff --git a/configs/android2_defconfig b/configs/android2_defconfig
index b079fa759..20866c32b 100644
--- a/configs/android2_defconfig
+++ b/configs/android2_defconfig
@@ -487,15 +487,12 @@ CONFIG_FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED=y
 # Options common to multiple modutils
 #
 # CONFIG_FEATURE_2_4_MODULES is not set
-# CONFIG_FEATURE_INSMOD_TRY_MMAP is not set
 # CONFIG_FEATURE_INSMOD_VERSION_CHECKING is not set
 # CONFIG_FEATURE_INSMOD_KSYMOOPS_SYMBOLS is not set
 # CONFIG_FEATURE_INSMOD_LOADINKMEM is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP_FULL is not set
 # CONFIG_FEATURE_CHECK_TAINTED_MODULE is not set
-# CONFIG_FEATURE_MODUTILS_ALIAS is not set
-# CONFIG_FEATURE_MODUTILS_SYMBOLS is not set
 CONFIG_DEFAULT_MODULES_DIR="/lib/modules"
 CONFIG_DEFAULT_DEPMOD_FILE="modules.dep"

diff --git a/configs/android_502_defconfig b/configs/android_502_defconfig
index 4273d3382..bdca9eebb 100644
--- a/configs/android_502_defconfig
+++ b/configs/android_502_defconfig
@@ -597,15 +597,12 @@ CONFIG_FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED=y
 # Options common to multiple modutils
 #
 # CONFIG_FEATURE_2_4_MODULES is not set
-# CONFIG_FEATURE_INSMOD_TRY_MMAP is not set
 # CONFIG_FEATURE_INSMOD_VERSION_CHECKING is not set
 # CONFIG_FEATURE_INSMOD_KSYMOOPS_SYMBOLS is not set
 # CONFIG_FEATURE_INSMOD_LOADINKMEM is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP_FULL is not set
 # CONFIG_FEATURE_CHECK_TAINTED_MODULE is not set
-# CONFIG_FEATURE_MODUTILS_ALIAS is not set
-# CONFIG_FEATURE_MODUTILS_SYMBOLS is not set
 CONFIG_DEFAULT_MODULES_DIR="/lib/modules"
 CONFIG_DEFAULT_DEPMOD_FILE="modules.dep"

diff --git a/configs/android_defconfig b/configs/android_defconfig
index b9489c456..6ef81750e 100644
--- a/configs/android_defconfig
+++ b/configs/android_defconfig
@@ -511,15 +511,12 @@ CONFIG_FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED=y
 # Options common to multiple modutils
 #
 # CONFIG_FEATURE_2_4_MODULES is not set
-# CONFIG_FEATURE_INSMOD_TRY_MMAP is not set
 # CONFIG_FEATURE_INSMOD_VERSION_CHECKING is not set
 # CONFIG_FEATURE_INSMOD_KSYMOOPS_SYMBOLS is not set
 # CONFIG_FEATURE_INSMOD_LOADINKMEM is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP is not set
 # CONFIG_FEATURE_INSMOD_LOAD_MAP_FULL is not set
 # CONFIG_FEATURE_CHECK_TAINTED_MODULE is not set
-# CONFIG_FEATURE_MODUTILS_ALIAS is not set
-#

[PATCH] modprobe-small: move lsmod code out of modprobe_main()

[Kang-Che Sung]

Having lsmod code inside modprobe_main() makes some of the applet name
checking code awkward. Besides, this make busybox x86_64 binary a few
bytes smaller. :)

Signed-off-by: Kang-Che Sung 

function  old new   delta
lsmod_main  -  23 +23
modprobe_main 599 564 -35
---
(add/remove: 1/0 grow/shrink: 0/1 up/down: 23/-35) Total: -12 bytes
---
 modutils/modprobe-small.c | 44 
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index 49c06d759..325f8376b 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -24,12 +24,12 @@
 //config: help
 //config:  Check if the module is already loaded.

+//applet:IF_LSMOD(   IF_MODPROBE_SMALL(APPLET(lsmod,BB_DIR_SBIN,
BB_SUID_DROP)))
 //applet:IF_MODPROBE(IF_MODPROBE_SMALL(APPLET(modprobe, BB_DIR_SBIN,
BB_SUID_DROP)))
 //   APPLET_ODDNAME:namemain
location suid_type help
 //applet:IF_DEPMOD(IF_MODPROBE_SMALL(APPLET_ODDNAME(depmod, modprobe,
BB_DIR_SBIN, BB_SUID_DROP, depmod)))
 //applet:IF_INSMOD(IF_MODPROBE_SMALL(APPLET_ODDNAME(insmod, modprobe,
BB_DIR_SBIN, BB_SUID_DROP, insmod)))
-//applet:IF_LSMOD(IF_MODPROBE_SMALL( APPLET_ODDNAME(lsmod,  modprobe,
BB_DIR_SBIN, BB_SUID_DROP, lsmod)))
-//applet:IF_RMMOD(IF_MODPROBE_SMALL( APPLET_ODDNAME(rmmod,  modprobe,
BB_DIR_SBIN, BB_SUID_DROP, rmmod)))
+//applet:IF_RMMOD( IF_MODPROBE_SMALL(APPLET_ODDNAME(rmmod,  modprobe,
BB_DIR_SBIN, BB_SUID_DROP, rmmod)))

 //kbuild:lib-$(CONFIG_MODPROBE_SMALL) += modprobe-small.o

@@ -59,7 +59,27 @@

 #define DEPFILE_BB CONFIG_DEFAULT_DEPMOD_FILE".bb"

-#define MOD_APPLET_CNT (ENABLE_MODPROBE + ENABLE_DEPMOD +
ENABLE_INSMOD + ENABLE_LSMOD + ENABLE_RMMOD)
+//usage:#if ENABLE_MODPROBE_SMALL
+
+//usage:#define lsmod_trivial_usage
+//usage:   ""
+//usage:#define lsmod_full_usage "\n\n"
+//usage:   "List loaded kernel modules"
+
+//usage:#endif
+
+#if ENABLE_LSMOD
+int lsmod_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
+int lsmod_main(int argc UNUSED_PARAM, char **argv UNUSED_PARAM)
+{
+ xprint_and_close_file(xfopen_for_read("/proc/modules"));
+ return EXIT_SUCCESS;
+}
+#endif
+
+/* Num of applets that use modprobe_main() entry point. */
+/* lsmod is not here. */
+#define MOD_APPLET_CNT (ENABLE_MODPROBE + ENABLE_DEPMOD +
ENABLE_INSMOD + ENABLE_RMMOD)

 /* Do not bother if MODPROBE_SMALL=y but no applets selected. */
 /* The rest of the file is in this if block. */
@@ -69,7 +89,6 @@
 #define is_modprobe (ENABLE_MODPROBE && (ONLY_APPLET || applet_name[0] == 'm'))
 #define is_depmod   (ENABLE_DEPMOD   && (ONLY_APPLET || applet_name[0] == 'd'))
 #define is_insmod   (ENABLE_INSMOD   && (ONLY_APPLET || applet_name[0] == 'i'))
-#define is_lsmod(ENABLE_LSMOD&& (ONLY_APPLET || applet_name[0] == 'l'))
 #define is_rmmod(ENABLE_RMMOD&& (ONLY_APPLET || applet_name[0] == 'r'))

 enum {
@@ -890,11 +909,6 @@ The following options are useful for people
managing distributions:
 //usage:#define depmod_trivial_usage NOUSAGE_STR
 //usage:#define depmod_full_usage ""

-//usage:#define lsmod_trivial_usage
-//usage:   ""
-//usage:#define lsmod_full_usage "\n\n"
-//usage:   "List loaded kernel modules"
-
 //usage:#define insmod_trivial_usage
 //usage: "FILE" IF_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE("
[SYMBOL=VALUE]...")
 //usage:#define insmod_full_usage "\n\n"
@@ -922,12 +936,6 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
  struct utsname uts;
  IF_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE(char *options = NULL;)

- /* are we lsmod? -> just dump /proc/modules */
- if (is_lsmod) {
- xprint_and_close_file(xfopen_for_read("/proc/modules"));
- return EXIT_SUCCESS;
- }
-
  INIT_G();

  /* Prevent ugly corner cases with no modules at all */
@@ -940,11 +948,7 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
  uname(&uts); /* never fails */

  /* depmod? */
- if ((MOD_APPLET_CNT == 2 && ENABLE_DEPMOD && ENABLE_LSMOD)
- /* ^^^"only depmod and lsmod is configured"^^ */
- /* note: we already know here it is not lsmod (handled before) */
- || is_depmod
- ) {
+ if (is_depmod) {
  /* Supported:
  * -n: print result to stdout
  * -a: process all modules (default)
-- 
2.11.0
From 2426a862471ffd5572296d1c67f4e38ff0125565 Mon Sep 17 00:00:00 2001
From: Kang-Che Sung 
Date: Tue, 31 Jan 2017 17:06:43 +0800
Subject: [PATCH] modprobe-small: move lsmod code out of modprobe_main()

Having lsmod code inside modprobe_main() makes some of the applet name
chec

Re: Android Port for Busybox?

[Kang-Che Sung]

On Tue, Jan 31, 2017 at 4:50 AM, Sriram V  wrote:
> I am using Android kitkat. I was compiling busybox for android and i
> get the following error when i select ifplugd.
>
> ifplugd.c:38:23: fatal error: linux/mii.h: No such file or directory
>
> I am compiling with an Android toolchain in android environment. I
> also tried with ndk and i get the same error as well.
>
> I need this application for my usage. Can anyone point me to the right
> sources which fixes the error.

I don't get it. Android uses Linux kernel, so for your Android toolchain it
should come with the  header that you mentioned.
Otherwise, how come does it include other Linux headers successfully
while for this one it fails?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

modprobe (small vs big) behavior inconsistency

[Kang-Che Sung]

I just discovered an inconsistency between the two modprobe
implementations (modprobe-small and the "big" modprobe).

It's about "checking whether module has already loaded" part.
Both implementations open and parse /proc/modules file,
however, the "small" version will read the "Live", "Loading" or
"Unloading" states of the modules, while the "big" version totally
ignores it.

So is this inconsistency a sign of a bug?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: ifplugd shell script not working

[Kang-Che Sung]

On Tue, Jan 31, 2017 at 10:49 PM, Sriram V  wrote:
> 1. ifplugd is a command which accepts a shell script as an argument.
> 2. if the link is up/down. It is supposed to invoke the shell script.
> 3. The code does a vfork and exec (2) system call.
> 4. I find that vfork is successful. However exec on a shell script
> (with executable permissions does) not work.
> 5. I get the error code as 2. which is no such file. I have given
> absolute pathname as well.
> 6. Is there a problem on arm system to use exec(2) on a shell script.
>
>
> # busybox ifplugd -nI
> ifplugd(eth0): started: BusyBox v1.27.0.git (2017-01-31 17:58:49 IST)
> ifplugd(eth0): using SIOCETHTOOL detection mode
> ifplugd(eth0): link is up
> ifplugd(eth0): executing '/etc/ifplugd/ifplugd.action eth0 up' (<<<
> This is just a print before calling vfork and exec >>>)
> ifplugd(eth0): exit code: 255
>
The exit code 255 here could mean _any_ kind of execvp error.
(To be precise, execvp(2) actually returns -1 upon error, which is got
"translated" by ifplugd so that you see exit code 255). It's not limited
to "no such file" case, so how can you be sure it's a "no such file" error?

Also, you said you intend to run a shell script. Have you built a shell in
BusyBox? I would be surprised if you made such a newbie mistake.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] modutils: remove redundant "select PLATFORM_LINUX" configs

[Kang-Che Sung]

It is enough to have only applets' configs select PLATFORM_LINUX.

Signed-off-by: Kang-Che Sung 
---
 modutils/Config.src | 11 ---
 modutils/lsmod.c|  1 -
 modutils/modprobe.c |  1 -
 3 files changed, 13 deletions(-)

diff --git a/modutils/Config.src b/modutils/Config.src
index 1aee57ee7..36a0a7225 100644
--- a/modutils/Config.src
+++ b/modutils/Config.src
@@ -8,7 +8,6 @@ menu "Linux Module Utilities"
 config MODPROBE_SMALL
  bool "Simplified modutils"
  default y
- select PLATFORM_LINUX
  help
   Simplified modutils.

@@ -44,7 +43,6 @@ config FEATURE_2_4_MODULES
  bool "Support version 2.2/2.4 Linux kernels"
  default n
  depends on (INSMOD || RMMOD || LSMOD) && !MODPROBE_SMALL
- select PLATFORM_LINUX
  help
   Support module loading for 2.2.x and 2.4.x Linux kernels.
   This increases size considerably. Say N unless you plan
@@ -54,7 +52,6 @@ config FEATURE_INSMOD_TRY_MMAP
  bool "Try to load module from a mmap'ed area"
  default n
  depends on INSMOD && !MODPROBE_SMALL
- select PLATFORM_LINUX
  help
   This option causes module loading code to try to mmap
   module first. If it does not work (for example,
@@ -71,7 +68,6 @@ config FEATURE_INSMOD_VERSION_CHECKING
  bool "Enable module version checking"
  default n
  depends on FEATURE_2_4_MODULES && (INSMOD || MODPROBE)
- select PLATFORM_LINUX
  help
   Support checking of versions for modules. This is used to
   ensure that the kernel and module are made for each other.
@@ -80,7 +76,6 @@ config FEATURE_INSMOD_KSYMOOPS_SYMBOLS
  bool "Add module symbols to kernel symbol table"
  default n
  depends on FEATURE_2_4_MODULES && (INSMOD || MODPROBE)
- select PLATFORM_LINUX
  help
   By adding module symbols to the kernel symbol table, Oops messages
   occuring within kernel modules can be properly debugged. By enabling
@@ -92,7 +87,6 @@ config FEATURE_INSMOD_LOADINKMEM
  bool "In kernel memory optimization (uClinux only)"
  default n
  depends on FEATURE_2_4_MODULES && (INSMOD || MODPROBE)
- select PLATFORM_LINUX
  help
   This is a special uClinux only memory optimization that lets insmod
   load the specified kernel module directly into kernel space, reducing
@@ -103,7 +97,6 @@ config FEATURE_INSMOD_LOAD_MAP
  bool "Enable insmod load map (-m) option"
  default n
  depends on FEATURE_2_4_MODULES && INSMOD
- select PLATFORM_LINUX
  help
   Enabling this, one would be able to get a load map
   output on stdout. This makes kernel module debugging
@@ -115,7 +108,6 @@ config FEATURE_INSMOD_LOAD_MAP_FULL
  bool "Symbols in load map"
  default y
  depends on FEATURE_INSMOD_LOAD_MAP && !MODPROBE_SMALL
- select PLATFORM_LINUX
  help
   Without this option, -m will only output section
   load map. With this option, -m will also output
@@ -125,7 +117,6 @@ config FEATURE_CHECK_TAINTED_MODULE
  bool "Support tainted module checking with new kernels"
  default y
  depends on (LSMOD || FEATURE_2_4_MODULES) && !MODPROBE_SMALL
- select PLATFORM_LINUX
  help
   Support checking for tainted modules. These are usually binary
   only modules that will make the linux-kernel list ignore your
@@ -136,7 +127,6 @@ config FEATURE_MODUTILS_ALIAS
  bool "Support module.aliases file"
  default y
  depends on (DEPMOD || MODPROBE) && !MODPROBE_SMALL
- select PLATFORM_LINUX
  help
   Generate and parse modules.alias containing aliases for bus
   identifiers:
@@ -153,7 +143,6 @@ config FEATURE_MODUTILS_SYMBOLS
  bool "Support module.symbols file"
  default y
  depends on (DEPMOD || MODPROBE) && !MODPROBE_SMALL
- select PLATFORM_LINUX
  help
   Generate and parse modules.symbols containing aliases for
   symbol_request() kernel calls, such as:
diff --git a/modutils/lsmod.c b/modutils/lsmod.c
index 9ab49f35b..24e5d35b9 100644
--- a/modutils/lsmod.c
+++ b/modutils/lsmod.c
@@ -18,7 +18,6 @@
 //config: bool "Pretty output"
 //config: default y
 //config: depends on LSMOD && !MODPROBE_SMALL
-//config: select PLATFORM_LINUX
 //config: help
 //config:  This option makes output format of lsmod adjusted to
 //config:  the format of module-init-tools for Linux kernel 2.6.
diff --git a/modutils/modprobe.c b/modutils/modprobe.c
index 09e3de6c3..cbec43888 100644
--- a/modutils/modprobe.c
+++ b/modutils/modprobe.c
@@ -19,7 +19,6 @@
 //config: bool "Blacklist support"
 //config: default y
 //config: depends on MODPROBE && !MODPROBE_SMALL
-//config: select PLATFORM_LINUX
 //config: help
 //config:  Say 'y' here to enable support for the 'blacklist' command in
 //config:  modprobe.conf. This prevents the alias resolver to resolve
-- 
2.11.0
From 10a54e192e9c1886b8d5a1dbb5eb46fe3f3a6a5e Mon Sep 17 00:00:00 2001
From: Kang-Che Sung 
Date: Tue, 31 Jan 2017 14:27:01 +0800
Subject: [PATCH] modutils: remove redundant "select PLATF

[PATCH] cmdline module options can be disabled on "big" modutils

[Kang-Che Sung]

Allow module options on command line to be disabled on "big" modutils.

Config FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE is renamed to
FEATURE_CMDLINE_MODULE_OPTIONS and no longer depends on !MODPROBE_SMALL

(I'm not sure if disabling this is useful on "big" modutils, but at
least the macro can serve as a marker and ensure both implementations
of same feature have consistent behavior.)

Signed-off-by: Kang-Che Sung 
---
 modutils/Config.src   |  8 
 modutils/insmod.c |  6 +++---
 modutils/modprobe-small.c | 23 ---
 modutils/modprobe.c   |  8 +++-
 modutils/modutils.c   |  2 ++
 modutils/modutils.h   |  4 
 6 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/modutils/Config.src b/modutils/Config.src
index f2448c914..a15cce518 100644
--- a/modutils/Config.src
+++ b/modutils/Config.src
@@ -37,6 +37,14 @@ INSERT

 comment "Options common to multiple modutils"

+config FEATURE_CMDLINE_MODULE_OPTIONS
+ bool "Accept module options on modprobe command line"
+ default y
+ depends on INSMOD || MODPROBE
+ help
+  Allow insmod and modprobe take module options from the applets'
+  command line.
+
 config FEATURE_2_4_MODULES
  bool "Support version 2.2/2.4 Linux kernels"
  default n
diff --git a/modutils/insmod.c b/modutils/insmod.c
index f2c70e16f..8526979eb 100644
--- a/modutils/insmod.c
+++ b/modutils/insmod.c
@@ -27,9 +27,9 @@

 //usage:#if !ENABLE_MODPROBE_SMALL
 //usage:#define insmod_trivial_usage
-//usage: IF_FEATURE_2_4_MODULES("[OPTIONS] MODULE ")
-//usage: IF_NOT_FEATURE_2_4_MODULES("FILE ")
-//usage: "[SYMBOL=VALUE]..."
+//usage: IF_FEATURE_2_4_MODULES("[OPTIONS] MODULE")
+//usage: IF_NOT_FEATURE_2_4_MODULES("FILE")
+//usage: IF_FEATURE_CMDLINE_MODULE_OPTIONS(" [SYMBOL=VALUE]...")
 //usage:#define insmod_full_usage "\n\n"
 //usage:   "Load kernel module"
 //usage: IF_FEATURE_2_4_MODULES( "\n"
diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index 325f8376b..04242634b 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -10,13 +10,6 @@

 /* config MODPROBE_SMALL is defined in Config.src to ensure better
"make config" order */

-//config:config FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE
-//config: bool "Accept module options on modprobe command line"
-//config: default y
-//config: depends on MODPROBE_SMALL && (INSMOD || MODPROBE)
-//config: help
-//config:  Allow insmod and modprobe take module options from command line.
-//config:
 //config:config FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED
 //config: bool "Skip loading of already loaded modules"
 //config: default y
@@ -690,7 +683,7 @@ static int rmmod(const char *filename)
  * NB: also called by depmod with bogus name "/",
  * just in order to force modprobe.dep.bb creation.
 */
-#if !ENABLE_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE
+#if !ENABLE_FEATURE_CMDLINE_MODULE_OPTIONS
 #define process_module(a,b) process_module(a)
 #define cmdline_options ""
 #endif
@@ -735,7 +728,7 @@ static int process_module(char *name, const char
*cmdline_options)
  options = xmalloc_open_read_close(opt_filename, NULL);
  if (options)
  replace(options, '\n', ' ');
-#if ENABLE_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE
+#if ENABLE_FEATURE_CMDLINE_MODULE_OPTIONS
  if (cmdline_options) {
  /* NB: cmdline_options always have one leading ' '
  * (see main()), we remove it here */
@@ -910,7 +903,7 @@ The following options are useful for people
managing distributions:
 //usage:#define depmod_full_usage ""

 //usage:#define insmod_trivial_usage
-//usage: "FILE" IF_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE("
[SYMBOL=VALUE]...")
+//usage: "FILE" IF_FEATURE_CMDLINE_MODULE_OPTIONS(" [SYMBOL=VALUE]...")
 //usage:#define insmod_full_usage "\n\n"
 //usage:   "Load kernel module"

@@ -920,7 +913,7 @@ The following options are useful for people
managing distributions:
 //usage:   "Unload kernel modules"

 //usage:#define modprobe_trivial_usage
-//usage: "[-rq] MODULE"
IF_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE(" [SYMBOL=VALUE]...")
+//usage: "[-rq] MODULE" IF_FEATURE_CMDLINE_MODULE_OPTIONS(" [SYMBOL=VALUE]...")
 //usage:#define modprobe_full_usage "\n\n"
 //usage:   " -r Remove MODULE"
 //usage: "\n -q Quiet"
@@ -934,7 +927,7 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
  int exitcode;
 #endif
  struct utsname uts;
- IF_FEATURE_MODPROBE_SMALL_OPTIONS_ON_CMDLINE(char *options = NULL;)
+ IF_FEATURE_CMDLINE_MODULE_OPTIONS(char *options = NULL;)

  INIT_G();

@@ -998,7 +991,7 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
  if (!ONLY_APPLET)
  option_mask

[PATCH] modutils: fix config options dependency (2)

[Kang-Che Sung]

- The modprobe-small implementation of rmmod no longer chdir's to
  "/lib/modules/`uname -r`" as it was not necessary for rmmod's
  operation. (And it no longer need to die if such modules directory
  doesn't exist.)
- Configs DEFAULT_MODULES_DIR and DEFAULT_DEPMOD_FILE no longer depend
  on MODPROBE_SMALL as the latter may not enable depmod or modprobe
  that requires these configs.
- Clarify DEFAULT_DEPMOD_FILE's description regarding the ".bb" name
  suffix.

Signed-off-by: Kang-Che Sung 
---
 modutils/Config.src   | 10 +++---
 modutils/modprobe-small.c | 36 +++-
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/modutils/Config.src b/modutils/Config.src
index a15cce518..d0bae2ea3 100644
--- a/modutils/Config.src
+++ b/modutils/Config.src
@@ -159,7 +159,7 @@ config FEATURE_MODUTILS_SYMBOLS
 config DEFAULT_MODULES_DIR
string "Default directory containing modules"
default "/lib/modules"
-   depends on DEPMOD || MODPROBE || MODPROBE_SMALL || MODINFO
+   depends on DEPMOD || MODPROBE || MODINFO
help
  Directory that contains kernel modules.
  Defaults to "/lib/modules"
@@ -167,9 +167,13 @@ config DEFAULT_MODULES_DIR
 config DEFAULT_DEPMOD_FILE
string "Default name of modules.dep"
default "modules.dep"
-   depends on DEPMOD || MODPROBE || MODPROBE_SMALL || MODINFO
+   depends on DEPMOD || MODPROBE || MODINFO
help
  Filename that contains kernel modules dependencies.
- Defaults to "modules.dep"
+ Defaults to "modules.dep".
+ If you configured the "simplified modutils" (MODPROBE_SMALL), a
+ ".bb" suffix will be added after this name. Do not specify ".bb"
+ here unless you intend your depmod or modprobe to work on
+ "modules.dep.bb.bb" or such.
 
 endmenu
diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index 04242634b..a3ba846a7 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -84,6 +84,22 @@ int lsmod_main(int argc UNUSED_PARAM, char **argv 
UNUSED_PARAM)
 #define is_insmod   (ENABLE_INSMOD   && (ONLY_APPLET || applet_name[0] == 'i'))
 #define is_rmmod(ENABLE_RMMOD&& (ONLY_APPLET || applet_name[0] == 'r'))
 
+/* Applets that need to chdir to /lib/modules/`uname -r` */
+/* Note: the last conditional is used when all 4 applets are configured, and
+ * employs an ASCII trick so that one check would suffice:
+ * 'm' -> 01101101
+ * 'd' -> 01100100
+ * 'i' -> 01101001
+ * 'r' -> 01110010('d' & 'm' & '\x1f') == '\x04' -> 0100
+ *.^.. note this bit! */
+#define is_depmod_or_modprobe ((ENABLE_MODPROBE || ENABLE_DEPMOD) \
+   && ((!ENABLE_INSMOD && !ENABLE_RMMOD) \
+   || (!ENABLE_MODPROBE && applet_name[0] == 'd') \
+   || (!ENABLE_DEPMOD   && applet_name[0] == 'm') \
+   || (!ENABLE_INSMOD   && applet_name[0] != 'r') \
+   || (!ENABLE_RMMOD&& applet_name[0] != 'i') \
+   || ((applet_name[0] & ('d' & 'm' & '\x1f')) != 0)))
+
 enum {
OPT_q = (1 << 0), /* be quiet */
OPT_r = (1 << 1), /* module removal instead of loading */
@@ -923,23 +939,30 @@ The following options are useful for people managing 
distributions:
 int modprobe_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int modprobe_main(int argc UNUSED_PARAM, char **argv)
 {
+#if ENABLE_DEPMOD || ENABLE_MODPROBE
+   struct utsname uts;
+#endif
 #if ENABLE_MODPROBE || ENABLE_INSMOD || ENABLE_RMMOD
int exitcode;
+# if ENABLE_FEATURE_CMDLINE_MODULE_OPTIONS
+   char *options = NULL;
+# endif
 #endif
-   struct utsname uts;
-   IF_FEATURE_CMDLINE_MODULE_OPTIONS(char *options = NULL;)
 
INIT_G();
 
/* Prevent ugly corner cases with no modules at all */
modinfo = xzalloc(sizeof(modinfo[0]));
 
-   if (!is_insmod) {
+#if ENABLE_DEPMOD || ENABLE_MODPROBE
+   if (is_depmod_or_modprobe) {
/* Goto modules directory */
xchdir(CONFIG_DEFAULT_MODULES_DIR);
+   uname(&uts); /* never fails */
}
-   uname(&uts); /* never fails */
+#endif
 
+#if ENABLE_DEPMOD
/* depmod? */
if (is_depmod) {
/* Supported:
@@ -971,6 +994,7 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
process_module((char*)"/", NULL);
return !wrote_dep_bb_ok;
}
+#endif /* DEPMOD */
 
 #if ENABLE_MODPROBE || ENABLE_INSMOD || ENABLE_RMMOD
/* modprobe, insmod, rmmod require 

[PATCH] Reorder modutils config options & fix yet more dependency

[Kang-Che Sung]

- modprobe can indirectly benefit from FEATURE_2_4_MODULES and
  FEATURE_INSMOD_TRY_MAP options.
- The position of config FEATURE_INSMOD_TRY_MMAP prevented some other
  config options from indenting under FEATURE_2_4_MODULES. Reorder to
  fix this.
- FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED is now moved to
  Config.src under "Common options" section. (I wished to edit this
  config so that it also work with "big" modutils, but it's not done at
  the moment. Sorry.)

Signed-off-by: Kang-Che Sung 
---
 modutils/Config.src   | 43 +--
 modutils/modprobe-small.c | 10 ++
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/modutils/Config.src b/modutils/Config.src
index d0bae2ea3..5f0b0cec4 100644
--- a/modutils/Config.src
+++ b/modutils/Config.src
@@ -45,31 +45,22 @@ config FEATURE_CMDLINE_MODULE_OPTIONS
  Allow insmod and modprobe take module options from the applets'
  command line.
 
+config FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED
+   bool "Skip loading of already loaded modules"
+   default y
+   depends on MODPROBE_SMALL && (DEPMOD || INSMOD || MODPROBE)
+   help
+ Check if the module is already loaded.
+
 config FEATURE_2_4_MODULES
bool "Support version 2.2/2.4 Linux kernels"
default n
-   depends on (INSMOD || RMMOD || LSMOD) && !MODPROBE_SMALL
+   depends on (INSMOD || LSMOD || MODPROBE || RMMOD) && !MODPROBE_SMALL
help
  Support module loading for 2.2.x and 2.4.x Linux kernels.
  This increases size considerably. Say N unless you plan
  to run ancient kernels.
 
-config FEATURE_INSMOD_TRY_MMAP
-   bool "Try to load module from a mmap'ed area"
-   default n
-   depends on INSMOD && !MODPROBE_SMALL
-   help
- This option causes module loading code to try to mmap
- module first. If it does not work (for example,
- it does not work for compressed modules), module will be read
- (and unpacked if needed) into a memory block allocated by malloc.
-
- The only case when mmap works but malloc does not is when
- you are trying to load a big module on a very memory-constrained
- machine. Malloc will momentarily need 2x as much memory as mmap.
-
- Choosing N saves about 250 bytes of code (on 32-bit x86).
-
 config FEATURE_INSMOD_VERSION_CHECKING
bool "Enable module version checking"
default n
@@ -113,7 +104,7 @@ config FEATURE_INSMOD_LOAD_MAP
 config FEATURE_INSMOD_LOAD_MAP_FULL
bool "Symbols in load map"
default y
-   depends on FEATURE_INSMOD_LOAD_MAP && !MODPROBE_SMALL
+   depends on FEATURE_INSMOD_LOAD_MAP
help
  Without this option, -m will only output section
  load map. With this option, -m will also output
@@ -129,6 +120,22 @@ config FEATURE_CHECK_TAINTED_MODULE
  support request.
  This option is required to support GPLONLY modules.
 
+config FEATURE_INSMOD_TRY_MMAP
+   bool "Try to load module from a mmap'ed area"
+   default n
+   depends on (INSMOD || MODPROBE) && !MODPROBE_SMALL
+   help
+ This option causes module loading code to try to mmap
+ module first. If it does not work (for example,
+ it does not work for compressed modules), module will be read
+ (and unpacked if needed) into a memory block allocated by malloc.
+
+ The only case when mmap works but malloc does not is when
+ you are trying to load a big module on a very memory-constrained
+ machine. Malloc will momentarily need 2x as much memory as mmap.
+
+ Choosing N saves about 250 bytes of code (on 32-bit x86).
+
 config FEATURE_MODUTILS_ALIAS
bool "Support module.aliases file"
default y
diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index a3ba846a7..0e9e86f08 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -8,14 +8,8 @@
  * Licensed under GPLv2, see file LICENSE in this source tree.
  */
 
-/* config MODPROBE_SMALL is defined in Config.src to ensure better "make 
config" order */
-
-//config:config FEATURE_MODPROBE_SMALL_CHECK_ALREADY_LOADED
-//config:  bool "Skip loading of already loaded modules"
-//config:  default y
-//config:  depends on MODPROBE_SMALL && (DEPMOD || INSMOD || MODPROBE)
-//config:  help
-//config:Check if the module is already loaded.
+/* modprobe-small configs are defined in Config.src to ensure better
+ * "make config" order */
 
 //applet:IF_LSMOD(   IF_MODPROBE_SMALL(APPLET(lsmod,BB_DIR_SBIN, 
BB_SUID_DROP)))
 //applet:IF_MODPROBE(IF_MODPROBE_SMALL(APPLET(modprobe, BB_DIR_SBIN, 
BB_SUID_DROP)))
-- 
2.11.0

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

Can we bring 'test' applet's help back?

[Kang-Che Sung]

This is just an idea.

After 'test' and '[' become separate, individually selectable applets, it
could be possible to bring the help of '[' and '[[' back after they got
removed in commit de5edadee2dca2896492f97ab3a56e389305e74d
("special-case {true,false,test} --help")

I mean that "test --help" will still display nothing, but "[ --help" without
the closing "]" or "[[ --help" without "]]" can show the help text.

So, is this a good idea?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Replace int -> uint to avoid signed integer overflow

[Kang-Che Sung]

On Mon, Feb 6, 2017 at 8:52 AM, Rob Landley  wrote:
>
> On 02/01/2017 12:35 PM, Rostislav Skudnov wrote:
>> An example of such an error (should be compiled with DEBUG_SANITIZE):
>>
>> runtime error: left shift of 1 by 31 places cannot be represented in
>> type 'int'
>
> Sure it can. We know exactly what bit pattern that represents in two's
> complement, which has been the ubiquitous representation of negative
> numbers since 1964.
>
> Is this another "may be used uninitialized" thing where the compiler's
> complaining about something that can't happen? Or have the C++ deveopers
> who took over implementation of the C compiler broken yet another thing
> that's universally worked since the 1960s because they want to extend
> "undefined behavior" until C is as much of a minefield of special case
> memorization as C++ is?
>
>> Signed-off-by: Rostislav Skudnov 
>> ---
>>  archival/libarchive/decompress_bunzip2.c | 6 +++---
>>  libbb/crc32.c| 2 +-
>>  libbb/getopt32.c | 4 ++--
>>  libbb/pw_encrypt.c   | 2 +-
>>  miscutils/rx.c   | 2 +-
>>  5 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/archival/libarchive/decompress_bunzip2.c 
>> b/archival/libarchive/decompress_bunzip2.c
>> index fe5953d..4fb989c 100644
>> --- a/archival/libarchive/decompress_bunzip2.c
>> +++ b/archival/libarchive/decompress_bunzip2.c
>> @@ -134,7 +134,7 @@ static unsigned get_bits(bunzip_data *bd, int 
>> bits_wanted)
>>
>>   /* Avoid 32-bit overflow (dump bit buffer to top of output) */
>>   if (bit_count >= 24) {
>> - bits = bd->inbufBits & ((1 << bit_count) - 1);
>> + bits = bd->inbufBits & ((1U << bit_count) - 1);
>
> What's an archive input that actually fails? What's an example of a
> processor machine language that doesn't produce 0x8000 for 1<<31?
>
> Where does this actually cause a problem?
>

I think at least this silences a "undefined behavior" warning, and that's
enough for the patch. Sure, many CPU architectures out there use two's
complement, but that's not mandated in the C standard.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] modutils: fix config options dependency (2)

[Kang-Che Sung]

On Mon, Feb 6, 2017 at 2:04 AM, Denys Vlasenko  wrote:
> Applied a simpler version of it. Thanks
>
> On Wed, Feb 1, 2017 at 10:22 AM, Kang-Che Sung  wrote:
>> - The modprobe-small implementation of rmmod no longer chdir's to
>>   "/lib/modules/`uname -r`" as it was not necessary for rmmod's
>>   operation. (And it no longer need to die if such modules directory
>>   doesn't exist.)
>> - Configs DEFAULT_MODULES_DIR and DEFAULT_DEPMOD_FILE no longer depend
>>   on MODPROBE_SMALL as the latter may not enable depmod or modprobe
>>   that requires these configs.
>> - Clarify DEFAULT_DEPMOD_FILE's description regarding the ".bb" name
>>   suffix.
>>

Thank you, but I hope you understand why I propose the not-so-simple route in
the patch. Especially regarding the use of is_depmod_or_modprobe macro.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Replace int -> uint to avoid signed integer overflow

[Kang-Che Sung]

On Mon, Feb 6, 2017 at 3:22 PM, Rob Landley  wrote:
> On 02/05/2017 09:10 PM, Kang-Che Sung wrote:
>> On Mon, Feb 6, 2017 at 8:52 AM, Rob Landley  wrote:
>>> What's an archive input that actually fails? What's an example of a
>>> processor machine language that doesn't produce 0x8000 for 1<<31?
>>>
>>> Where does this actually cause a problem?
>>
>> I think at least this silences a "undefined behavior" warning, and that's
>> enough for the patch.
>
> So the problem _is_ the warning? There's no real issue, just a noisy
> compiler?

Technically yes. It's "-Wshift-overflow=2" although it's off by default and
little people care about these days.

>
> Which is why we have more than one standard: Posix, LP64, LSB, ELF...
> Posix requires a machine to handle two's complement here:
>
>   http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdint.h.html
>

And C++14.

> Could you give me an example of an architecture from this century that
> _doesn't_ use two's complement? The IBM S/360 (introduced in 1964) used
> two's complement. The first minicomputer (PDP-8 in 1965) used two's
> complement. The first microcomputer (Altair, 1975) used two's complement
> (altair basic manual page 28 says so)...

I have never been arguing about this. Your point stands. Yes, there are no
architectures today that would use anything else than two's complement
for signed arithmetic. But it's the standard that wished to be more
conservative. (C standard has not yet been updated despite POSIX and
C++14)

Now I don't like to argue with this further. This patch breaks nothing changes
nothing in machine code, just to silence a warning. If it's me then
I'll merge it
because it wouldn't hurt anyway.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Replace int -> uint to avoid signed integer overflow

[Kang-Che Sung]

In case that Rob Landley isn't convinced, there is another argument
supporting casting to unsigned before bit shifting:

There are little cases that left shifting to sign bit is actually useful.
AFAIK, the use of 1<<31 cases are no other than
1. intended to represent a signed INTn_MIN constant,
2. intended to do arithmetic (signed) left shift, without checking
overflow (a bad coding practice and potential bug),
3. doing shift on a bitfield or unsigned int but forgot to cast it to
unsigned.

It seems to me that this bunzip2 case is case 3, and for readability
I support changing the variable types to unsigned where they
should be.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] modutils: fix config options dependency (2)

[Kang-Che Sung]

On Tue, Feb 7, 2017 at 4:56 AM, Denys Vlasenko  wrote:
> On Mon, Feb 6, 2017 at 4:38 AM, Kang-Che Sung  wrote:
>> Thank you, but I hope you understand why I propose the not-so-simple route in
>> the patch. Especially regarding the use of is_depmod_or_modprobe macro.
>
> Not really. If you would explain it, it might increase chances of it
> being accepted. Since I had to guess, I guessed "it probably saves a few bytes
> of code at the cost of many more #ifdefs.

Yes, it's to save a few bytes in the generated machine code.
(Although I think of this later I might be putting to many #ifdefs
than necessary.)

Here this is sufficient:

#define is_depmod_or_modprobe \
((ENABLE_MODPROBE || ENABLE_DEPMOD) \
&& ((!ENABLE_INSMOD && !ENABLE_RMMOD) \
|| (!ENABLE_MODPROBE && is_depmod) \
|| ((applet_name[0] & '\x04') != 0)))

With comments:

#define is_depmod_or_modprobe \
/* If neither modprobe or depmod are configured, false */ \
((ENABLE_MODPROBE || ENABLE_DEPMOD) \
/* If there's nothing but modprobe or depmod, true */ \
&& ((!ENABLE_INSMOD && !ENABLE_RMMOD) \
/* This helps compiler to merge the block with is_depmod one below */ \
|| (!ENABLE_MODPROBE && is_depmod) \
/* Otherwise determine 'd' or 'm' using ASCII and bitwise trick */ \
/* ("and" and "je" in x86/x64, same size as "cmp" and "je" in binary; */
/* it's smaller than two "cmp"s and "je"s) */ \
|| ((applet_name[0] & '\x4') != 0)))
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Replace int -> uint to avoid signed integer overflow

[Kang-Che Sung]

(Forgot to CC the mailing list on the last sent mail. Resent to CC the
mailing list)

On Tue, Feb 7, 2017 at 6:59 AM, Rob Landley  wrote:
> My attitude towards false positives may have been influenced by people
> running static checkers against toybox and submitting long spreadsheets
> of results, which I've spent hours going through and writing up my
> analysis of each hit, although I spent a lot more effort on it the first
> few times it happened:
>
> http://lists.landley.net/pipermail/toybox-landley.net/2014-February/003280.html
>
> My reluctance to "fix" non-bugs may have been informed by that sort of
> thing. SOMEBODY has to be able to reproduce the issue, or at least
> feasibly explain what they think might occur.

Sorry for off-topic reply, but in your mail pointed by the URL above, I should
say that some of the cppcheck complaints make sense. Well, that's what
cppcheck was supposed to do. It can't assume your libc version, and for
portability's sake it will better warn you than being silent.

Take the "scanf without field width limits can crash with huge data" for
example, I hope you have read this cppcheck's developer's reply:
https://stackoverflow.com/questions/9292861/

In short, you are right to argue that it's libc's fault and not yours,
however, do note that POSIX [1] didn't yet specify integer overflow
behavior for scanf. If a libc implementation uses strtol() internally for
scanf integer parsing then you're safe, but I'll suggest switching to strtol
if possible to ensure a defined behavior instead.

(And take advantage of endptr:
sscanf(str, "%u%n", &u, &count);
/* equivalent */
u = strtol(str,&endptr,10), count = (int)(endptr-str);
)

If you wish to ignore the complaint, that's your choice.
I'm just to show an idea.

[1] (http://pubs.opengroup.org/onlinepubs/9699919799/functions/scanf.html)
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] modutils: fix config options dependency (2)

[Kang-Che Sung]

On Tue, Feb 7, 2017 at 11:21 PM, Denys Vlasenko
 wrote:
> On Tue, Feb 7, 2017 at 4:38 AM, Kang-Che Sung  wrote:
>> On Tue, Feb 7, 2017 at 4:56 AM, Denys Vlasenko  
>> wrote:
>>>
>>> Not really. If you would explain it, it might increase chances of it
>>> being accepted. Since I had to guess, I guessed "it probably saves a few 
>>> bytes
>>> of code at the cost of many more #ifdefs.
>>
>> Yes, it's to save a few bytes in the generated machine code.
>> (Although I think of this later I might be putting to many #ifdefs
>> than necessary.)
>>
>> Here this is sufficient:
>>
>> #define is_depmod_or_modprobe \
>> ((ENABLE_MODPROBE || ENABLE_DEPMOD) \
>> && ((!ENABLE_INSMOD && !ENABLE_RMMOD) \
>> || (!ENABLE_MODPROBE && is_depmod) \
>> || ((applet_name[0] & '\x04') != 0)))
>
> How about this?
>
> -   if (is_depmod || is_modprobe) {
> +   if ((MOD_APPLET_CNT == 2 && ENABLE_MODPROBE && ENABLE_DEPMOD)
> +|| is_depmod || is_modprobe
> +   ) {

You can imagine how it will evaluate for this configuration:

#define ENABLE_DEPMOD 1
#define ENABLE_MODPROBE 1
#define ENABLE_INSMOD 1
/* ENABLE_RMMOD is not set */

MOD_APPLET_CNT==3

(MOD_APPLET_CNT == 2) evaluates to false (0)
is_depmod expands to (applet_name[0]=='d')
is_modprobe expands to (applet_name[0]=='m')

So the whole expression becomes
(applet_name[0]=='d' || applet_name[0]=='m')

I would prefer the smaller comparison: (applet_name[0] != 'i')
Or the equal sized one: (applet_name[0] & '\4')

This is the MOD_APPLET_CNT==3 case that I've thought of.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] modutils: fix config options dependency (2)

[Kang-Che Sung]

On Tue, Feb 7, 2017 at 11:40 PM, Denys Vlasenko
 wrote:
> On Tue, Feb 7, 2017 at 4:38 PM, Kang-Che Sung  wrote:
>> On Tue, Feb 7, 2017 at 11:21 PM, Denys Vlasenko
>>  wrote:
>>>
>>> How about this?
>>>
>>> -   if (is_depmod || is_modprobe) {
>>> +   if ((MOD_APPLET_CNT == 2 && ENABLE_MODPROBE && ENABLE_DEPMOD)
>>> +|| is_depmod || is_modprobe
>>> +   ) {
>>
>> You can imagine how it will evaluate for this configuration:
>>
>> #define ENABLE_DEPMOD 1
>> #define ENABLE_MODPROBE 1
>> #define ENABLE_INSMOD 1
>> /* ENABLE_RMMOD is not set */
>>
>> MOD_APPLET_CNT==3
>>
>> (MOD_APPLET_CNT == 2) evaluates to false (0)
>> is_depmod expands to (applet_name[0]=='d')
>> is_modprobe expands to (applet_name[0]=='m')
>>
>> So the whole expression becomes
>> (applet_name[0]=='d' || applet_name[0]=='m')
>>
>> I would prefer the smaller comparison: (applet_name[0] != 'i')
>> Or the equal sized one: (applet_name[0] & '\4')
>>
>> This is the MOD_APPLET_CNT==3 case that I've thought of.
>
> The difference would be +1 2-byte asm instruction on x86.

No, actually it's +2 2-byte asm instructions, one for "cmp" and one for "je".

I just tried compiling it in gcc-6.3.0, and disassembled to read which
instructions are added. And so I'm sure this is the result.

Attaching part of my "objdump -d -r modprobe-small.o" output

Disassembly of section .text.modprobe_main:
[---truncated---]
  34: 48 8b 05 00 00 00 00 mov0x0(%rip),%rax# 3b

37: R_X86_64_PC32 applet_name-0x4
  3b: 8a 00 mov(%rax),%al
  3d: 3c 64 cmp$0x64,%al
  3f: 74 04 je 45 
  41: 3c 6d cmp$0x6d,%al
  43: 75 14 jne59 
  45: bf 00 00 00 00   mov$0x0,%edi
46: R_X86_64_32 .rodata.modprobe_main.str1.1 #
CONFIG_DEFAULT_MODULES_DIR
  4a: e8 00 00 00 00   callq  4f 
4b: R_X86_64_PC32 xchdir-0x4
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH 2/3] Update depmod & modprobe upstream help text in comments

[Kang-Che Sung]

No code changes.

Signed-off-by: Kang-Che Sung 
---
 modutils/modprobe-small.c | 38 +++---
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index 0ac39f66f..52765bc99 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -862,33 +862,39 @@ Usage: rmmod [-fhswvV] modulename ...
 should eventually fall to zero).
 
 # modprobe
-Usage: modprobe [-v] [-V] [-C config-file] [-n] [-i] [-q] [-b]
-[-o ] [ --dump-modversions ]  [parameters...]
+Usage: modprobe [-v] [-V] [-C config-file] [-d  ] [-n] [-i] [-q] \
+[-b] [-o ] [ --dump-modversions ]  [parameters...]
 modprobe -r [-n] [-i] [-v]  ...
 modprobe -l -t  [ -a  ...]
 
 # depmod --help
-depmod 3.4 -- part of module-init-tools
-depmod -[aA] [-n -e -v -q -V -r -u]
+depmod 3.13 -- part of module-init-tools
+depmod -[aA] [-n -e -v -q -V -r -u -w -m]
   [-b basedirectory] [forced_version]
-depmod [-n -e -v -q -r -u] [-F kernelsyms] module1.ko module2.ko ...
+depmod [-n -e -v -q -r -u -w] [-F kernelsyms] module1.ko module2.ko ...
 If no arguments (except options) are given, "depmod -a" is assumed.
 depmod will output a dependency list suitable for the modprobe utility.
 Options:
--a, --all   Probe all modules
--A, --quick Only does the work if there's a new module
--n, --show  Write the dependency file on stdout only
--e, --errsyms   Report not supplied symbols
--V, --version   Print the release version
--v, --verbose   Enable verbose mode
--h, --help  Print this usage message
+-a, --allProbe all modules
+-A, --quick  Only does the work if there's a new module
+-e, --errsymsReport not supplied symbols
+-m, --mapCreate the legacy map files
+-n, --show   Write the dependency file on stdout only
+-P, --symbol-prefix  Architecture symbol prefix
+-V, --versionPrint the release version
+-v, --verboseEnable verbose mode
+-w, --warn   Warn on duplicates
+-h, --help   Print this usage message
 The following options are useful for people managing distributions:
 -b basedirectory
 --basedir basedirectory
-Use an image of a module tree
+ Use an image of a module tree.
 -F kernelsyms
 --filesyms kernelsyms
-Use the file instead of the current kernel symbols
+ Use the file instead of the current kernel symbols.
+-E Module.symvers
+--symvers Module.symvers
+ Use Module.symvers file to check symbol versions.
 */
 
 //usage:#if ENABLE_MODPROBE_SMALL
@@ -950,10 +956,12 @@ int modprobe_main(int argc UNUSED_PARAM, char **argv)
 * -e: report any symbols which a module needs
 *  which are not supplied by other modules or the kernel
 * -F FILE: System.map (symbols for -e)
-* -q, -r, -u: noop?
+* -q, -r, -u: noop
 * Not supported:
 * -b BASEDIR: (TODO!) modules are in
 *  $BASEDIR/lib/modules/$VERSION
+* -m: create legacy "modules.*map" files (deprecated; in
+*  kmod's depmod, prints a warning message and continues)
 * -v: human readable deps to stdout
 * -V: version (don't want to support it - people may depend
 *  on it as an indicator of "standard" depmod)
-- 
2.11.0

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

[PATCH 3/3] modprobe-small: define and use DEPMOD_OPT_n (option mask)

[Kang-Che Sung]

Signed-off-by: Kang-Che Sung 
---
 modutils/modprobe-small.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index 52765bc99..43625490b 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -79,6 +79,7 @@ int lsmod_main(int argc UNUSED_PARAM, char **argv 
UNUSED_PARAM)
 #define is_rmmod(ENABLE_RMMOD&& (ONLY_APPLET || applet_name[0] == 'r'))
 
 enum {
+   DEPMOD_OPT_n = (1 << 0), /* dry-run, print to stdout */
OPT_q = (1 << 0), /* be quiet */
OPT_r = (1 << 1), /* module removal instead of loading */
 };
@@ -477,7 +478,7 @@ static int start_dep_bb_writeout(void)
int fd;
 
/* depmod -n: write result to stdout */
-   if (applet_name[0] == 'd' && (option_mask32 & 1))
+   if (is_depmod && (option_mask32 & DEPMOD_OPT_n))
return STDOUT_FILENO;
 
fd = open(DEPFILE_BB".new", O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 
0644);
-- 
2.11.0

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

[PATCH 1/3] modprobe-small: document '-n' in depmod usage

[Kang-Che Sung]

Signed-off-by: Kang-Che Sung 
---
 modutils/modprobe-small.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
index 431b8aeb2..41fd89172 100644
--- a/modutils/modprobe-small.c
+++ b/modutils/modprobe-small.c
@@ -893,8 +894,11 @@ The following options are useful for people managing 
distributions:
 
 //usage:#if ENABLE_MODPROBE_SMALL
 
-//usage:#define depmod_trivial_usage NOUSAGE_STR
-//usage:#define depmod_full_usage ""
+//usage:#define depmod_trivial_usage "[-n]"
+//usage:#define depmod_full_usage "\n\n"
+//usage:   "Generate modules.dep.bb"
+//usage: "\n"
+//usage: "\n   -n  Dry run: print file to stdout"
 
 //usage:#define insmod_trivial_usage
 //usage:   "FILE" IF_FEATURE_CMDLINE_MODULE_OPTIONS(" [SYMBOL=VALUE]...")
-- 
2.11.0

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

[PATCH] modprobe: simplify code (printf in do_modprobe())

[Kang-Che Sung]

Signed-off-by: Kang-Che Sung 
---
 modutils/modprobe.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/modutils/modprobe.c b/modutils/modprobe.c
index a6224fa63..58e8b5a15 100644
--- a/modutils/modprobe.c
+++ b/modutils/modprobe.c
@@ -466,10 +466,9 @@ static int do_modprobe(struct module_entry *m)
 #endif
 
if (option_mask32 & OPT_SHOW_DEPS) {
-   printf(options ? "insmod %s/%s/%s %s\n"
-   : "insmod %s/%s/%s\n",
+   printf("insmod %s/%s/%s %s\n",
CONFIG_DEFAULT_MODULES_DIR, G.uts.release, fn,
-   options);
+   options ? options : "");
free(options);
continue;
}
-- 
2.11.0

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

Re: [PATCH 3/3] modprobe-small: define and use DEPMOD_OPT_n (option mask)

[Kang-Che Sung]

On Fri, Feb 10, 2017 at 9:09 PM, Xabier Oneca  --  xOneca
 wrote:
> Hello,
>
> 2017-02-09 15:48 GMT+01:00 Kang-Che Sung :
>> Signed-off-by: Kang-Che Sung 
>> ---
>>  modutils/modprobe-small.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/modutils/modprobe-small.c b/modutils/modprobe-small.c
>> index 52765bc99..43625490b 100644
>> --- a/modutils/modprobe-small.c
>> +++ b/modutils/modprobe-small.c
>> @@ -79,6 +79,7 @@ int lsmod_main(int argc UNUSED_PARAM, char **argv 
>> UNUSED_PARAM)
>>  #define is_rmmod(ENABLE_RMMOD&& (ONLY_APPLET || applet_name[0] == 
>> 'r'))
>>
>>  enum {
>> +   DEPMOD_OPT_n = (1 << 0), /* dry-run, print to stdout */
>
> Why not call it OPT_n? Is it used somewere else?
>

I have considered using OPT_n before, but I also think about the '-n'
option being implemented for modprobe/insmod/rmmod in the future,
and that way the option bits will conflict. (At least I tried to hack the
code to implement '-n'. It's --dry-run, so implement it won't be difficult,
although the code won't be neat.)

depmod doesn't use the same getopt32 call as modprobe/insmod/
rmmod's call. So it's better for the OPT constants be separate as well.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox