Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Very good!!! So is it time to re-apply zlib patch to u-boot main tree, together Alessandro's patch? Wolfgang, what's your position? Best Regards, Giuseppe -Original Message- From: u-boot-boun...@lists.denx.de [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Stefan Roese Sent: Tuesday, July 28, 2009 8:31 AM To: Alessandro Rubini Cc: u-boot@lists.denx.de; rhabarber1...@web.de Subject: Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer On Monday 27 July 2009 18:40:31 Alessandro Rubini wrote: The entry point of an image can be 0 (like on PowerPC), so allow next_out to be NULL in inflate() Signed-off-by: Alessandro Rubini rub...@unipv.it --- Stefan Roese: I'll try to uncompress to something != 0 tomorrow on PPC. You are right. I naively thought my arm has RAM at 0 like PPC, but entry point of image is not really 0. Yes, this works: Tested-by: Stefan Roese s...@denx.de Thanks. Best regards, Stefan = DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Dear Giuseppe CONDORELLI, In message 001601ca1012$e0f72730$c0818...@st.com you wrote: Very good!!! So is it time to re-apply zlib patch to u-boot main tree, together Alessandro's patch? Wolfgang, what's your position? Actually I'm waiting for a new patch that integrates that fix. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Just about every computer on the market today runs Unix, except the Mac (and nobody cares about it). - Bill Joy 6/21/85 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Dear Wolfgang, Actually I'm waiting for a new patch that integrates that fix. Maybe I've not understood. Do you mean you are waiting a new zlib patch [v4] that includes Alessandro's patch, to apply only one patch containing all? Best Regards, Giuseppe -Original Message- From: Wolfgang Denk [mailto:w...@denx.de] Sent: Wednesday, July 29, 2009 8:24 AM To: Giuseppe CONDORELLI Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer Dear Giuseppe CONDORELLI, In message 001601ca1012$e0f72730$c0818...@st.com you wrote: Very good!!! So is it time to re-apply zlib patch to u-boot main tree, together Alessandro's patch? Wolfgang, what's your position? Actually I'm waiting for a new patch that integrates that fix. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Just about every computer on the market today runs Unix, except the Mac (and nobody cares about it). - Bill Joy 6/21/85 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Dear Giuseppe CONDORELLI, In message 001701ca1015$dbf7fae0$c0818...@st.com you wrote: Actually I'm waiting for a new patch that integrates that fix. Maybe I've not understood. Do you mean you are waiting a new zlib patch [v4] that includes Alessandro's patch, to apply only one patch containing all? Yes. It makes no sense to add a known to be broken patch and fix it in a second commit, as this leaves only a state in the tree where attempts to bisect any issues run into problems. So please submit a new, fixed patch, that really works. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Applying computer technology is simply finding the right wrench to pound in the correct screw. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Dear rhabarber1848, In message h4orm0$l1...@ger.gmane.org you wrote: Actually I'm waiting for a new patch that integrates that fix. please do not commit the zlib-1.2.3 patch in its current state, although Alessandro's patch fixes on bug there is still the problem that the WATCHDOG_RESET() calls from gunzip.c #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) s.outcb = (cb_func)WATCHDOG_RESET; #else s.outcb = Z_NULL; #endif/* CONFIG_HW_WATCHDOG */ are silently ignored in the new zlib code. My C knowledge is insufficient to solve this problem. In its current state it will break zlib decompression on the machines I am taking care of. Agreed - that should be working, too, before we pull that patch into mainline. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Perfection is reached, not when there is no longer anything to add, but when there is no longer anything to take away. - Antoine de Saint-Exupery ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Hi rhabarber1848, please could you test latest two zlib patches I have sent few minutes ago? NOTE: the first one is waiting for approval due to its big size. Thanks, I hope these patches will solve your issue. G. -Original Message- From: u-boot-boun...@lists.denx.de [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Wolfgang Denk Sent: Wednesday, July 29, 2009 9:12 AM To: rhabarber1...@web.de Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer Dear rhabarber1848, In message h4orm0$l1...@ger.gmane.org you wrote: Actually I'm waiting for a new patch that integrates that fix. please do not commit the zlib-1.2.3 patch in its current state, although Alessandro's patch fixes on bug there is still the problem that the WATCHDOG_RESET() calls from gunzip.c #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) s.outcb = (cb_func)WATCHDOG_RESET; #else s.outcb = Z_NULL; #endif/* CONFIG_HW_WATCHDOG */ are silently ignored in the new zlib code. My C knowledge is insufficient to solve this problem. In its current state it will break zlib decompression on the machines I am taking care of. Agreed - that should be working, too, before we pull that patch into mainline. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Perfection is reached, not when there is no longer anything to add, but when there is no longer anything to take away. - Antoine de Saint-Exupery ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Hi rhabarber1848,
please could you test latest two zlib patches I have sent few minutes ago?
Unfortunately you add outcb() only in inflatestart and inflateend. IT should
also go in the main loop.
Being an out-callback with arguments, it should be called during copy to
output, with proper arguments (and in that form it would be a fix to
the official zlib).
I tried this one, which is a quick hack (no arguments passed, so not
suitable for upstream zlib in any case). Moreover, one of those two
should be sufficient, but I don't know which one exactly. I'm lazy so I
won't trace program flow in detail.
I think rhabarber should test with this addition too and select which one
is the good one.
(formatted for git-am for quick testing, even thought he talk is irrelevant
as a commit message).
ps: please note that your addition of outcb() has white-space inconsistency with
zlib.c (which is not u-boot style while those lines are).
/alessandro
---
lib_generic/zlib.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/lib_generic/zlib.c b/lib_generic/zlib.c
index 6a78dc9..66b18eb 100644
--- a/lib_generic/zlib.c
+++ b/lib_generic/zlib.c
@@ -1129,6 +1129,10 @@ unsigned out;
state = (struct inflate_state FAR *)strm-state;
+/* call watchdog_reset if needed (addition for U-Boot) */
+if (strm-outcb != Z_NULL)
+(*strm-outcb)(Z_NULL, 0);
+
/* if it hasn't been done already, allocate space for the window */
if (state-window == Z_NULL) {
state-window = (unsigned char FAR *)
@@ -1741,6 +1745,8 @@ int flush;
Tracev((stderr, inflate: codes ok\n));
state-mode = LEN;
case LEN:
+if (strm-outcb != Z_NULL) /* for watchdog (U-Boot) */
+(*strm-outcb)(Z_NULL, 0);
if (have = 6 left = 258) {
RESTORE();
inflate_fast(strm, out);
--
1.6.0.2
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Hi,
Alessandro Rubini wrote:
Giuseppe Condorelli wrote:
Hi rhabarber1848,
please could you test latest two zlib patches I have sent few minutes
ago?
Unfortunately you add outcb() only in inflatestart and inflateend. IT
should also go in the main loop.
you are right. The two patches sent by Giuseppe do not solve the problem.
I think rhabarber should test with this addition too and select which one
is the good one.
Let's see:
--- a/lib_generic/zlib.c
+++ b/lib_generic/zlib.c
@@ -1129,6 +1129,10 @@ unsigned out;
state = (struct inflate_state FAR *)strm-state;
+/* call watchdog_reset if needed (addition for U-Boot) */
+if (strm-outcb != Z_NULL)
+(*strm-outcb)(Z_NULL, 0);
+
/* if it hasn't been done already, allocate space for the window */
if (state-window == Z_NULL) {
state-window = (unsigned char FAR *)
This patch does not fix the problem.
@@ -1741,6 +1745,8 @@ int flush;
Tracev((stderr, inflate: codes ok\n));
state-mode = LEN;
case LEN:
+if (strm-outcb != Z_NULL) /* for watchdog (U-Boot) */
+(*strm-outcb)(Z_NULL, 0);
if (have = 6 left = 258) {
RESTORE();
inflate_fast(strm, out);
This patch alone seems to solve the problem, only with it U-Boot could
decompress a Linux image, proven with 10 successful boot attempts. All the
other patches regarding WATCHDOG_RESET() can be ignored.
Tested with U-Boot 2008.09-rc1, the original zlib123-patch from Giuseppe +
your allow 0 as destination pointer-patch and the last patch from the
mail quoted above.
Cheers, rhabarber1848
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Hi rhbarber1848,
I've merged patches into one only and resent it to list (v5).
Please verify once more the issue is solved, to close it definitively.
Cheers and thanks,
Giuseppe
-Original Message-
From: u-boot-boun...@lists.denx.de [mailto:u-boot-boun...@lists.denx.de] On
Behalf Of rhabarber1848
Sent: Wednesday, July 29, 2009 12:33 PM
To: u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Hi,
Alessandro Rubini wrote:
Giuseppe Condorelli wrote:
Hi rhabarber1848,
please could you test latest two zlib patches I have sent few minutes
ago?
Unfortunately you add outcb() only in inflatestart and inflateend. IT
should also go in the main loop.
you are right. The two patches sent by Giuseppe do not solve the problem.
I think rhabarber should test with this addition too and select which one
is the good one.
Let's see:
--- a/lib_generic/zlib.c
+++ b/lib_generic/zlib.c
@@ -1129,6 +1129,10 @@ unsigned out;
state = (struct inflate_state FAR *)strm-state;
+/* call watchdog_reset if needed (addition for U-Boot) */
+if (strm-outcb != Z_NULL)
+(*strm-outcb)(Z_NULL, 0);
+
/* if it hasn't been done already, allocate space for the window */
if (state-window == Z_NULL) {
state-window = (unsigned char FAR *)
This patch does not fix the problem.
@@ -1741,6 +1745,8 @@ int flush;
Tracev((stderr, inflate: codes ok\n));
state-mode = LEN;
case LEN:
+if (strm-outcb != Z_NULL) /* for watchdog (U-Boot) */
+(*strm-outcb)(Z_NULL, 0);
if (have = 6 left = 258) {
RESTORE();
inflate_fast(strm, out);
This patch alone seems to solve the problem, only with it U-Boot could
decompress a Linux image, proven with 10 successful boot attempts. All the
other patches regarding WATCHDOG_RESET() can be ignored.
Tested with U-Boot 2008.09-rc1, the original zlib123-patch from Giuseppe +
your allow 0 as destination pointer-patch and the last patch from the
mail quoted above.
Cheers, rhabarber1848
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Alessandro Rubini wrote: The entry point of an image can be 0 (like on PowerPC), so allow next_out to be NULL in inflate() Hi, this patch solves the inflate() error but now a watchdog reset is triggered during decompression: ## Booting kernel from Legacy Image at 0020 ... Image Name: Linux-2.4.37.2-dbox2 Image Type: PowerPC Linux Kernel Image (gzip compressed) Data Size:772042 Bytes = 753.9 kB Load Address: Entry Point: Verifying Checksum ... OK Uncompressing Kernel Image ... debug: DDF: Calibrating delay loop... debug: DDF: 66.76 BogoMIPS debug: WATCHDOG RESET [...] The neither the old nor the new code incorporate WATCHDOG_RESET() so the new code seems to be slower than the old one which works. I have yet to dig through the zlib code to insert some WATCHDOG_RESET() here and there like I did with the LZMA code. Cheers, rhabarber1848 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Dear rhabarber1848, In message h4kq2v$eo...@ger.gmane.org you wrote: this patch solves the inflate() error but now a watchdog reset is triggered during decompression: ... The neither the old nor the new code incorporate WATCHDOG_RESET() so the new You are wrong. The urrent code installs WATCHDOG_RESET as s.outcb() callback; see function zunzip() in lib_generic/gunzip.c. code seems to be slower than the old one which works. I have yet to dig through the zlib code to insert some WATCHDOG_RESET() here and there like I did with the LZMA code. I think this is the wrong thing to do. Use the provided callback instead. I wonder why you do not create a patch between the old original version and what wd have in U-Boot now (which should clearly show what has been changed to adapt this code for U-Boot), and then apply (if necessary, manually) that patch to the current version? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de You ain't experienced... Well, nor are you. That's true. But the point is ... the point is ... the point is we've been not experienced for a lot longer than you. We've got a lot of experience of not having any experience. - Terry Pratchett, _Witches Abroad_ ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Wolfgang Denk wrote: I wonder why you do not create a patch between the old original version and what wd have in U-Boot now (which should clearly show what has been changed to adapt this code for U-Boot), and then apply (if necessary, manually) that patch to the current version? Hi Wolfgang, I did exactly that, I took rc1, downloaded the zlib-1.2.3 patch from the newsgroup[1] and added the patch by Alessandro[2]. The WATCHDOG_RESET code in gunzip.c is not touched by either patches, but I still get a WATCHDOG_RESET when the kernel image is decompressed with the new code. Though is seems that if (z-outcb != Z_NULL) (*z-outcb)(Z_NULL, 0); is not used anymore in zlib.c so the WATCHDOG_RESET calls are silently ignored. I will try to re-add them tomorrow. Besides this, rc1 is working fine here. Cheers, rhabarber1848 [1] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/64727 [2] http://article.gmane.org/gmane.comp.boot-loaders.u-boot/64969 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] zlib: allow 0 as destination pointer
Dear rhabarber1848, In message h4l1p8$d5...@ger.gmane.org you wrote: Though is seems that if (z-outcb != Z_NULL) (*z-outcb)(Z_NULL, 0); is not used anymore in zlib.c so the WATCHDOG_RESET calls are silently ignored. I will try to re-add them tomorrow. I see. Besides this, rc1 is working fine here. Thanks for the confirmation. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Pardon me for breathing, which I never do anyway so I don't know why I bother to say it, oh God, I'm so depressed. Here's another of those self-satisfied doors. Life! Don't talk to me about life. - Marvin the Paranoid Android ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
