[U-Boot] [PATCH] xyzModem.c Packet buffer dynamic allocate

[U-Boot] [PATCH] xyzModem.c Packet buffer dynamic allocate

[Yoshinori Sato]

Hello.

It changes reduce bss usage.
Ymodem receive buffer dinamic allocation.

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 common/cmd_load.c |    1 +
 common/xyzModem.c |   17 ++++++++++++++---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/common/cmd_load.c b/common/cmd_load.c
index dad0303..f144bf4 100644
--- a/common/cmd_load.c
+++ b/common/cmd_load.c
@@ -992,6 +992,7 @@ static ulong load_serial_ymodem (ulong offset)
 						  store_addr, res);
 				if (rc != 0) {
 					flash_perror (rc);
+					xyzModem_stream_close (&err);
 					return (~0);
 				}
 			} else
diff --git a/common/xyzModem.c b/common/xyzModem.c
index 7a46805..5af995d 100644
--- a/common/xyzModem.c
+++ b/common/xyzModem.c
@@ -58,6 +58,7 @@
 #include <xyzModem.h>
 #include <stdarg.h>
 #include <crc.h>
+#include <malloc.h>
 
 /* Assumption - run xyzModem protocol over the console port */
 
@@ -81,7 +82,7 @@ static struct
 #else
   int *__chan;
 #endif
-  unsigned char pkt[1024], *bufp;
+  unsigned char *pkt, *bufp;
   unsigned char blk, cblk, crc1, crc2;
   unsigned char next_blk;	/* Expected block */
   int len, mode, total_retries;
@@ -353,6 +354,7 @@ xyzModem_get_hdr (void)
   bool hdr_found = false;
   int i, can_total, hdr_chars;
   unsigned short cksum;
+  char *pktp;
 
   ZM_DEBUG (zm_new ());
   /* Find the start of a header */
@@ -433,13 +435,14 @@ xyzModem_get_hdr (void)
     }
   xyz.len = (c == SOH) ? 128 : 1024;
   xyz.bufp = xyz.pkt;
+  pktp = xyz.pkt;
   for (i = 0; i < xyz.len; i++)
     {
       res = CYGACC_COMM_IF_GETC_TIMEOUT (*xyz.__chan, &c);
       ZM_DEBUG (zm_save (c));
       if (res)
 	{
-	  xyz.pkt[i] = c;
+	  *pktp++ = c;
 	}
       else
 	{
@@ -489,9 +492,10 @@ xyzModem_get_hdr (void)
   else
     {
       cksum = 0;
+      pktp = xyz.pkt;
       for (i = 0; i < xyz.len; i++)
 	{
-	  cksum += xyz.pkt[i];
+	  cksum += *pktp++;
 	}
       if (xyz.crc1 != (cksum & 0xFF))
 	{
@@ -560,6 +564,11 @@ xyzModem_stream_open (connection_info_t * info, int *err)
   xyz.read_length = 0;
   xyz.file_length = 0;
 #endif
+  if (xyz.pkt == NULL) {
+    xyz.pkt = malloc(1024);
+    if (xyz.pkt == NULL)
+      return -1;
+  }
 
   CYGACC_COMM_IF_PUTC (*xyz.__chan, (xyz.crc_mode ? 'C' : NAK));
 
@@ -743,6 +752,8 @@ xyzModem_stream_close (int *err)
      xyz.crc_mode ? "CRC" : "Cksum", xyz.total_SOH, xyz.total_STX,
      xyz.total_CAN, xyz.total_retries);
   ZM_DEBUG (zm_flush ());
+  free(xyz.pkt);
+  xyz.pkt = NULL;
 }
 
 /* Need to be able to clean out the input buffer, so have to take the */
-- 
1.7.1

[U-Boot] [PATCH] xyzModem.c Packet buffer dynamic allocate

[Wolfgang Denk]

Dear Yoshinori Sato,

In message <87sk0q1mnf.wl%ysato@users.sourceforge.jp> you wrote:
> 
> It changes reduce bss usage.

... and increases the load on the malloc arean.

> Ymodem receive buffer dinamic allocation.

As far as I can see we increase the code size, and shift a fixed sized
buffer from bss to the maloc arene.

I don't consider this an improvement, but I may be overlooking
something.

Which advantages do you see with the changed code?

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: wd at denx.de
If the facts don't fit the theory, change the facts.
                                                   -- Albert Einstein

[U-Boot] [PATCH] xyzModem.c Packet buffer dynamic allocate

[Yoshinori Sato]

At Fri, 01 Oct 2010 09:57:49 +0200,
Wolfgang Denk wrote:
> 
> Dear Yoshinori Sato,
> 
> In message <87sk0q1mnf.wl%ysato@users.sourceforge.jp> you wrote:
> > 
> > It changes reduce bss usage.
> 
> ... and increases the load on the malloc arean.
> 
> > Ymodem receive buffer dinamic allocation.
> 
> As far as I can see we increase the code size, and shift a fixed sized
> buffer from bss to the maloc arene.
> 
> I don't consider this an improvement, but I may be overlooking
> something.
> 
> Which advantages do you see with the changed code?

My target have too small size RAM. 
I want reduced to data and bss.

It is limited to one usage in a static allocation. 
But dynamic allocation can use generically.
 
> 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: wd at denx.de
> If the facts don't fit the theory, change the facts.
>                                                    -- Albert Einstein

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

[U-Boot] [PATCH] xyzModem.c Packet buffer dynamic allocate

[Mike Frysinger]

On Fri, Oct 1, 2010 at 11:02 AM, Yoshinori Sato wrote:
> At Fri, 01 Oct 2010 09:57:49 +0200, Wolfgang Denk wrote:
>> Yoshinori Sato wrote:
>> > It changes reduce bss usage.
>>
>> ... and increases the load on the malloc arean.
>>
>> > Ymodem receive buffer dinamic allocation.
>>
>> As far as I can see we increase the code size, and shift a fixed sized
>> buffer from bss to the maloc arene.
>>
>> I don't consider this an improvement, but I may be overlooking
>> something.
>>
>> Which advantages do you see with the changed code?
>
> My target have too small size RAM.
> I want reduced to data and bss.
>
> It is limited to one usage in a static allocation.
> But dynamic allocation can use generically.

i feel like this is a much more uncommon usage scenario and is more
harmful than good to boards, and one which isnt specific to xyzModem.
perhaps we need a general tune config which says "prefer malloc over
bss" and this creates a set of helper macros which expand either into
malloc calls or nothing (since the storage is already in bss).
-mike