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

[Kang-Che Sung]

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

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

This is how I proposed letting the user change the compression level
when GZIP_LEVELS is turned off, when discussing about gzip default
compression level. This patch is a proof of concept of that.


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

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

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

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

[Kang-Che Sung]

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

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

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

Re: Some issues in examples/udhcp/simple.script

[Cristian Ionescu-Idbohrn]

On Thu, 5 Sep 2019, Rolf Eike Beer wrote:
> 
> Busybox ash:
> 
> # command ip
> -sh: command: not found
> # ip a
> 1: lo:  mtu 65536 qdisc noqueue qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00

You're probably missing:

CONFIG_ASH_CMDCMD=y

in your .config.


Cheers,

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

Re: Some issues in examples/udhcp/simple.script

[Rolf Eike Beer]

Am Donnerstag, 5. September 2019, 14:57:15 CEST schrieb Denys Vlasenko:
> On Wed, Aug 14, 2019 at 9:50 AM Rolf Eike Beer  wrote:
> > Hi all,
> > 
> > I have discovered some problems with examples/udhcp/simple.script. I know,
> > it's an example, and a simplified one. Most people will still just use it
> > because it works, so I would like to point out at least some things.
> > 
> > First, it does not really use ip. At least not if "command" does not
> > exist.
> 
> "command" is a mandatory shell builtin.
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
> 
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
> section "Command Search and Execution" specifically mentions it.
> 
> The reason for this mention is that its function
> can not reasonably be implemented as a separate external tool,
> since external tools don't know what functions are defined
> in current shell's invocation, thus "command FUNCNAME"
> won't work.
> 
> Since it's an internal builtin, using it should be a fastest way to test
> whether "ip" can be executed, without assuming a path
> ("test -x /usr/bin/ip") or forking a possibly external tool
> ("which ip").
> 
> Which shell do you use so that you don't have "command"?

Busybox ash:

# command ip
-sh: command: not found
# ip a
1: lo:  mtu 65536 qdisc noqueue qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
…

> > The difference between both cases is: ifconfig brings up the interface, ip
> > with this command does not. So if you are using ip this will just send
> > DHCP
> > request to a network interface that has link down, which will not get you
> > anywhere.
> 
> Yes, this is different. How about:
> 
> echo "Clearing IP addresses on $interface, upping it"
> if command -v ip >/dev/null; then
> ip addr flush dev $interface
> ip link set dev $interface up
> else
> ifconfig $interface 0.0.0.0
> fi

Which is basically what I'm using now, just different order of commands.

Thanks,

Eike
-- 
Rolf Eike Beer, emlix GmbH, http://www.emlix.com
Fon +49 551 30664-0, Fax +49 551 30664-11
Gothaer Platz 3, 37083 Göttingen, Germany
Sitz der Gesellschaft: Göttingen, Amtsgericht Göttingen HR B 3160
Geschäftsführung: Heike Jordan, Dr. Uwe Kracke – Ust-IdNr.: DE 205 198 055

emlix - smart embedded open source

signature.asc
Description: This is a digitally signed message part.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: Some issues in examples/udhcp/simple.script

[Denys Vlasenko]

On Wed, Aug 14, 2019 at 9:50 AM Rolf Eike Beer  wrote:
> Hi all,
>
> I have discovered some problems with examples/udhcp/simple.script. I know,
> it's an example, and a simplified one. Most people will still just use it
> because it works, so I would like to point out at least some things.
>
> First, it does not really use ip. At least not if "command" does not exist.

"command" is a mandatory shell builtin.

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html
section "Command Search and Execution" specifically mentions it.

The reason for this mention is that its function
can not reasonably be implemented as a separate external tool,
since external tools don't know what functions are defined
in current shell's invocation, thus "command FUNCNAME"
won't work.

Since it's an internal builtin, using it should be a fastest way to test
whether "ip" can be executed, without assuming a path
("test -x /usr/bin/ip") or forking a possibly external tool
("which ip").

Which shell do you use so that you don't have "command"?

> Fair enough, I know I have ip, so I just deleted the checks and the ifconfig
> lines.
>
> This makes it do nothing. As I found out the culprit is this:
>
> if command -v ip >/dev/null; then
> ip addr flush dev $interface
> else
> ifconfig $interface 0.0.0.0
> fi
>
> The difference between both cases is: ifconfig brings up the interface, ip
> with this command does not. So if you are using ip this will just send DHCP
> request to a network interface that has link down, which will not get you
> anywhere.

Yes, this is different. How about:

echo "Clearing IP addresses on $interface, upping it"
if command -v ip >/dev/null; then
ip addr flush dev $interface
ip link set dev $interface up
else
ifconfig $interface 0.0.0.0
fi
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

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

[Daniel Edgecumbe]

Well, not just the default behaviour.

It's probably nonsensical to assume that a specific compression level will
produce identical (or even similar) results across different implementations 
because
there are other possible variables, like the GZIP_FAST flag.

There is though a slight complication in that due to BusyBox being embedded
software we skip some checks. For example, with GZIP_LEVELS off, -1..9 flags are
just ignored transparently. So in that case scripts that rely on the compression
levels will fail anyway.

The patch set was more aimed at fixing some behaviours that I think aren't 
intuitive.
Specifically I think it's very strange that the default would go from -9 to -6 
when
levels are enabled. Much head scratching was had.

Daniel


On 05/09/2019 14.32, Kang-Che Sung wrote:
> Well, it seems that Denys merged the changes before I have the time to
> respond to comments, but anyway:
> 
> I actually disliked the argument about "de-facto standard" on compression
> levels. I think scripts should not depend on the default compression level,
> and should instead specify it explicitly. The reason is that you can not
> guarantee the 'gzip' on one machine is the same implementation on another
> (there are implementations that use more extensive search, for example,
> 7-zip and zopfli), it is just a coincidence that BusyBox gzip uses an
> implementation that's compatible with the most popular (zlib, I think), but
> it shouldn't be "guaranteed", let alone becoming a "standard" on
> implementaion or on the default behaviors.
> 
> I am not suggesting to change the patch now. I'm just mentioning that
> relying on default behavior of any program, for any script, is a bad idea
> in general. (E.g. Do specify -6 if your script expects it, even though it
> seems redundant, because user setting can always override the program
> defaults.)
> 

-- 
Daniel Edgecumbe | esotericnonsense
Kalix NO, Sverige | +358 46 584 2810
em...@esotericnonsense.com | https://esotericnonsense.com
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

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

[Kang-Che Sung]

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

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

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

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

[Denys Vlasenko]

Applied, thanks

On Mon, Sep 2, 2019 at 11:58 PM Daniel Edgecumbe
 wrote:
>
> Let's try once more, patches inline (new email client, gah!)
>
> 0001-gzip-default-level-with-ENABLE_FEATURE_GZIP_LEVELS-s.patch
>
> From 9d06f01e2805a5d6f1d775ceb651ae18ae2e1808 Mon Sep 17 00:00:00 2001
> From: Daniel Edgecumbe 
> Date: Mon, 2 Sep 2019 22:03:14 +0100
> Subject: [PATCH 1/3] gzip: default level with ENABLE_FEATURE_GZIP_LEVELS
>  should be 6
>
> Fixes an off-by-one that actually resulted in level 7 being used
> ---
>  archival/gzip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/archival/gzip.c b/archival/gzip.c
> index 17341de45..37db347b8 100644
> --- a/archival/gzip.c
> +++ b/archival/gzip.c
> @@ -,7 +,7 @@ int gzip_main(int argc UNUSED_PARAM, char **argv)
>  #if ENABLE_FEATURE_GZIP_LEVELS
> opt >>= (BBUNPK_OPTSTRLEN IF_FEATURE_GZIP_DECOMPRESS(+ 2) + 1); /* 
> drop cfkvq[dt]n bits */
> if (opt == 0)
> -   opt = 1 << 6; /* default: 6 */
> +   opt = 1 << 5; /* default: 6 */
> opt = ffs(opt >> 4); /* Maps -1..-4 to [0], -5 to [1] ... -9 to [5] */
> max_chain_length = 1 << gzip_level_config[opt].chain_shift;
> good_match   = gzip_level_config[opt].good;
> --
> 2.23.0
>
> 0002-gzip-set-compression-flags-correctly-as-per-standard.patch
>
> From 4280c9633b359dcbf2ddadcf33790b8690f81c82 Mon Sep 17 00:00:00 2001
> From: Daniel Edgecumbe 
> Date: Mon, 2 Sep 2019 22:05:26 +0100
> Subject: [PATCH 2/3] gzip: set compression flags correctly as per standard
>
> With this change and CONFIG_GZIP_FAST=2, CONFIG_FEATURE_GZIP_LEVELS=y,
>
> GNU gzip and BusyBox gzip now produce identical output at each compression
> level (excluding 1..3, as BusyBox does not implement these levels).
> ---
>  archival/gzip.c | 22 --
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/archival/gzip.c b/archival/gzip.c
> index 37db347b8..f13748aa1 100644
> --- a/archival/gzip.c
> +++ b/archival/gzip.c
> @@ -259,6 +259,7 @@ enum {
>
>  #if !ENABLE_FEATURE_GZIP_LEVELS
>
> +   comp_level = 9,
> max_chain_length = 4096,
>  /* To speed up deflation, hash chains are never searched beyond this length.
>   * A higher limit improves compression ratio but degrades the speed.
> @@ -334,10 +335,12 @@ struct globals {
>  #define head (G1.prev + WSIZE) /* hash head (see deflate.c) */
>
>  #if ENABLE_FEATURE_GZIP_LEVELS
> +   unsigned comp_level;
> unsigned max_chain_length;
> unsigned max_lazy_match;
> unsigned good_match;
> unsigned nice_match;
> +#define comp_level (G1.comp_level)
>  #define max_chain_length (G1.max_chain_length)
>  #define max_lazy_match   (G1.max_lazy_match)
>  #define good_match  (G1.good_match)
> @@ -1919,7 +1922,7 @@ static void bi_init(void)
>  /* 
> ===
>   * Initialize the "longest match" routines for a new file
>   */
> -static void lm_init(unsigned *flags16p)
> +static void lm_init(void)
>  {
> unsigned j;
>
> @@ -1927,8 +1930,6 @@ static void lm_init(unsigned *flags16p)
> memset(head, 0, HASH_SIZE * sizeof(*head));
> /* prev will be initialized on the fly */
>
> -   /* speed options for the general purpose bit flag */
> -   *flags16p |= 2; /* FAST 4, SLOW 2 */
> /* ??? reduce max_chain_length for binary files */
>
> //G1.strstart = 0; // globals are zeroed in pack_gzip()
> @@ -2076,10 +2077,16 @@ static void zip(void)
>
> bi_init();
> ct_init();
> -   deflate_flags = 0;  /* pkzip -es, -en or -ex equivalent */
> -   lm_init(_flags);
> +   lm_init();
>
> -   put_16bit(deflate_flags | 0x300); /* extra flags. OS id = 3 (Unix) */
> +   deflate_flags = 0x300; /* extra flags. OS id = 3 (Unix) */
> +#if ENABLE_FEATURE_GZIP_LEVELS
> +   /* Note that comp_levels < 4 do not exist in this version of gzip */
> +   if (comp_level == 9) {
> +   deflate_flags |= 0x02; /* SLOW flag */
> +   }
> +#endif
> +   put_16bit(deflate_flags);
>
> /* The above 32-bit misaligns outbuf (10 bytes are stored), flush it 
> */
> flush_outbuf_if_32bit_optimized();
> @@ -2224,6 +2231,9 @@ int gzip_main(int argc UNUSED_PARAM, char **argv)
> if (opt == 0)
> opt = 1 << 5; /* default: 6 */
> opt = ffs(opt >> 4); /* Maps -1..-4 to [0], -5 to [1] ... -9 to [5] */
> +
> +   comp_level = opt + 4;
> +
> max_chain_length = 1 << gzip_level_config[opt].chain_shift;
> good_match   = gzip_level_config[opt].good;
> max_lazy_match   = gzip_level_config[opt].lazy2 * 2;
> --
> 2.23.0
>
> 0003-gzip-set-default-compression-level-to-6-when-CONFIG_.patch
>
> From 12d30559486502feec4e2821b3ab45ae6139e7aa Mon Sep 17 00:00:00 2001
> From: Daniel Edgecumbe 
> Date: Mon, 2 Sep 2019 22:09:15 +0100
> Subject: [PATCH 3/3] gzip: set default compression