| Message ID | 1474312112-27835-10-git-send-email-thuth@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On 20/09/16 05:08, Thomas Huth wrote: > There is no need anymore to pass most of the arguments as > strings, we can use integers and pointers now instead. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > lib/libnet/libnet.code | 23 +++++++++-------------- > lib/libnet/netapps.h | 3 ++- > lib/libnet/netload.c | 21 ++++++++++++--------- > slof/fs/packages/obp-tftp.fs | 15 ++++----------- > 4 files changed, 27 insertions(+), 35 deletions(-) > > diff --git a/lib/libnet/libnet.code b/lib/libnet/libnet.code > index ab67fac..3602543 100644 > --- a/lib/libnet/libnet.code > +++ b/lib/libnet/libnet.code > @@ -2,20 +2,15 @@ > #include <netapps.h> > > PRIM(NET_X2d_LOAD) > - int slen = TOS.n; POP; > - char *arg = TOS.a; > - char *argvs[8]; > - int i, p; > - argvs[0] = arg; > - i = 1; > - for (p = 0; p < slen; p++) { > - if (arg[p] == ' ') { > - arg[p] = 0; > - argvs[i] = &arg[p + 1]; > - i++; > - } > - } > - TOS.n = netboot(i, argvs); > + int alen = TOS.n; POP; > + char *arg = TOS.a; POP; > + int blocksize = TOS.n; POP; > + int hugeload = TOS.n; POP; > + void *replybuf = TOS.a; POP; > + long maxlen = TOS.n; POP; > + void *loadaddr = TOS.a; > + TOS.n = netload(loadaddr, maxlen, replybuf, hugeload, blocksize, > + arg, alen); > MIRP > > PRIM(NET_X2d_PING) > diff --git a/lib/libnet/netapps.h b/lib/libnet/netapps.h > index 00b6318..91c1ebd 100644 > --- a/lib/libnet/netapps.h > +++ b/lib/libnet/netapps.h > @@ -18,7 +18,8 @@ > > struct filename_ip; > > -extern int netboot(int argc, char *argv[]); > +extern int netload(char *buffer, int len, char *ret_buffer, int huge_load, > + int block_size, char *args_fs, int alen); > extern int netsave(int argc, char *argv[]); > extern int ping(char *args_fs, int alen); > extern int dhcp(char *ret_buffer, struct filename_ip *fn_ip, > diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c > index 6429405..848609d 100644 > --- a/lib/libnet/netload.c > +++ b/lib/libnet/netload.c > @@ -386,14 +386,11 @@ static void seed_rng(uint8_t mac[]) > srand(seed); > } > > -int > -netboot(int argc, char *argv[]) > +int netload(char *buffer, int len, char *ret_buffer, int huge_load, > + int block_size, char *args_fs, int alen) > { > char buf[256]; > int rc; > - int len = strtol(argv[2], 0, 16); > - char *buffer = (char *) strtol(argv[1], 0, 16); > - char *ret_buffer = (char *) strtol(argv[3], 0, 16); > filename_ip_t fn_ip; > int fd_device; > tftp_err_t tftp_err; > @@ -403,8 +400,6 @@ netboot(int argc, char *argv[]) > 0x00, 0x00, 0x00, 0x00, > 0x00, 0x00, 0x00, 0x00, > 0x00, 0x00, 0x00, 0x00 }; > - int huge_load = strtol(argv[4], 0, 10); > - int32_t block_size = strtol(argv[5], 0, 10); > uint8_t own_mac[6]; > > puts("\n Initializing NIC"); > @@ -458,8 +453,16 @@ netboot(int argc, char *argv[]) > > seed_rng(own_mac); > > - if (argc > 6) { > - parse_args(argv[6], &obp_tftp_args); > + if (alen > 0) { > + char args[256]; > + if (alen > sizeof(args) - 1) { > + puts("ERROR: Parameter string is too long."); > + return -7; > + } > + /* Convert forth string into NUL-terminated C-string */ > + strncpy(args, args_fs, alen); > + args[alen] = 0; > + parse_args(args, &obp_tftp_args); > if(obp_tftp_args.bootp_retries - rc < DEFAULT_BOOT_RETRIES) > obp_tftp_args.bootp_retries = DEFAULT_BOOT_RETRIES; > else > diff --git a/slof/fs/packages/obp-tftp.fs b/slof/fs/packages/obp-tftp.fs > index 30070a6..2e6bcfc 100644 > --- a/slof/fs/packages/obp-tftp.fs > +++ b/slof/fs/packages/obp-tftp.fs > @@ -33,20 +33,13 @@ INSTANCE VARIABLE ciregs-buffer > my-parent ihandle>phandle node>path encode-string > s" bootpath" set-chosen > > - \ Generate arg string for snk like > - \ "netboot load-addr length filename" > - (u.) s" netboot " 2swap $cat s" 60000000 " $cat > + 60000000 ( addr maxlen ) > > \ Allocate 1720 bytes to store the BOOTP-REPLY packet > - 6B8 alloc-mem dup >r (u.) $cat > - huge-tftp-load @ IF s" 1 " ELSE s" 0 " THEN $cat > - \ Add desired TFTP-Blocksize as additional argument > - s" 1432 " $cat > + 6B8 alloc-mem dup >r ( addr maxlen replybuf ) > + huge-tftp-load @ d# 1428 ( addr maxlen replybuf hugetftp blocksize ) This 1432 -> 1428 change does not seem to belong to this patch, why is this change? > \ Add OBP-TFTP Bootstring argument, e.g. "10.128.0.1,bootrom.bin,10.128.40.1" > - my-args $cat > - \ Zero-terminate string > - s" " $cat 2dup + 1 - 0 swap c! > - > + my-args > net-load dup 0< IF drop 0 THEN > > \ Restore to old client interface register >
On 10.10.2016 04:46, Alexey Kardashevskiy wrote: > On 20/09/16 05:08, Thomas Huth wrote: >> There is no need anymore to pass most of the arguments as >> strings, we can use integers and pointers now instead. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> lib/libnet/libnet.code | 23 +++++++++-------------- >> lib/libnet/netapps.h | 3 ++- >> lib/libnet/netload.c | 21 ++++++++++++--------- >> slof/fs/packages/obp-tftp.fs | 15 ++++----------- >> 4 files changed, 27 insertions(+), 35 deletions(-) ... >> diff --git a/slof/fs/packages/obp-tftp.fs b/slof/fs/packages/obp-tftp.fs >> index 30070a6..2e6bcfc 100644 >> --- a/slof/fs/packages/obp-tftp.fs >> +++ b/slof/fs/packages/obp-tftp.fs >> @@ -33,20 +33,13 @@ INSTANCE VARIABLE ciregs-buffer >> my-parent ihandle>phandle node>path encode-string >> s" bootpath" set-chosen >> >> - \ Generate arg string for snk like >> - \ "netboot load-addr length filename" >> - (u.) s" netboot " 2swap $cat s" 60000000 " $cat >> + 60000000 ( addr maxlen ) >> >> \ Allocate 1720 bytes to store the BOOTP-REPLY packet >> - 6B8 alloc-mem dup >r (u.) $cat >> - huge-tftp-load @ IF s" 1 " ELSE s" 0 " THEN $cat >> - \ Add desired TFTP-Blocksize as additional argument >> - s" 1432 " $cat >> + 6B8 alloc-mem dup >r ( addr maxlen replybuf ) >> + huge-tftp-load @ d# 1428 ( addr maxlen replybuf hugetftp blocksize ) > > > This 1432 -> 1428 change does not seem to belong to this patch, why is this > change? 1428 is the right maximum size that should be used for TFTP transfers (otherwise there might be problems if networking is using VLANs underneath, IIRC). The old 1432 value is silently fixed by the code in tftp.c: /* Preferred blocksize - used as option for the read request */ if (_blocksize < 8) _blocksize = 8; else if (_blocksize > MAX_BLOCKSIZE) _blocksize = MAX_BLOCKSIZE; with MAX_BLOCKSIZE defined to 1428. I guess I should have mentioned that in the commit message... or if you prefer, I can also do it in a separate patch instead? Thomas
On 11/10/16 23:39, Thomas Huth wrote: > On 10.10.2016 04:46, Alexey Kardashevskiy wrote: >> On 20/09/16 05:08, Thomas Huth wrote: >>> There is no need anymore to pass most of the arguments as >>> strings, we can use integers and pointers now instead. >>> >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>> --- >>> lib/libnet/libnet.code | 23 +++++++++-------------- >>> lib/libnet/netapps.h | 3 ++- >>> lib/libnet/netload.c | 21 ++++++++++++--------- >>> slof/fs/packages/obp-tftp.fs | 15 ++++----------- >>> 4 files changed, 27 insertions(+), 35 deletions(-) > ... >>> diff --git a/slof/fs/packages/obp-tftp.fs b/slof/fs/packages/obp-tftp.fs >>> index 30070a6..2e6bcfc 100644 >>> --- a/slof/fs/packages/obp-tftp.fs >>> +++ b/slof/fs/packages/obp-tftp.fs >>> @@ -33,20 +33,13 @@ INSTANCE VARIABLE ciregs-buffer >>> my-parent ihandle>phandle node>path encode-string >>> s" bootpath" set-chosen >>> >>> - \ Generate arg string for snk like >>> - \ "netboot load-addr length filename" >>> - (u.) s" netboot " 2swap $cat s" 60000000 " $cat >>> + 60000000 ( addr maxlen ) >>> >>> \ Allocate 1720 bytes to store the BOOTP-REPLY packet >>> - 6B8 alloc-mem dup >r (u.) $cat >>> - huge-tftp-load @ IF s" 1 " ELSE s" 0 " THEN $cat >>> - \ Add desired TFTP-Blocksize as additional argument >>> - s" 1432 " $cat >>> + 6B8 alloc-mem dup >r ( addr maxlen replybuf ) >>> + huge-tftp-load @ d# 1428 ( addr maxlen replybuf hugetftp blocksize ) >> >> >> This 1432 -> 1428 change does not seem to belong to this patch, why is this >> change? > > 1428 is the right maximum size that should be used for TFTP transfers > (otherwise there might be problems if networking is using VLANs > underneath, IIRC). > The old 1432 value is silently fixed by the code in tftp.c: > > /* Preferred blocksize - used as option for the read request */ > if (_blocksize < 8) > _blocksize = 8; > else if (_blocksize > MAX_BLOCKSIZE) > _blocksize = MAX_BLOCKSIZE; > > with MAX_BLOCKSIZE defined to 1428. > > I guess I should have mentioned that in the commit message... or if you > prefer, I can also do it in a separate patch instead? Up to you, a note in the commit log is fine for me.
diff --git a/lib/libnet/libnet.code b/lib/libnet/libnet.code index ab67fac..3602543 100644 --- a/lib/libnet/libnet.code +++ b/lib/libnet/libnet.code @@ -2,20 +2,15 @@ #include <netapps.h> PRIM(NET_X2d_LOAD) - int slen = TOS.n; POP; - char *arg = TOS.a; - char *argvs[8]; - int i, p; - argvs[0] = arg; - i = 1; - for (p = 0; p < slen; p++) { - if (arg[p] == ' ') { - arg[p] = 0; - argvs[i] = &arg[p + 1]; - i++; - } - } - TOS.n = netboot(i, argvs); + int alen = TOS.n; POP; + char *arg = TOS.a; POP; + int blocksize = TOS.n; POP; + int hugeload = TOS.n; POP; + void *replybuf = TOS.a; POP; + long maxlen = TOS.n; POP; + void *loadaddr = TOS.a; + TOS.n = netload(loadaddr, maxlen, replybuf, hugeload, blocksize, + arg, alen); MIRP PRIM(NET_X2d_PING) diff --git a/lib/libnet/netapps.h b/lib/libnet/netapps.h index 00b6318..91c1ebd 100644 --- a/lib/libnet/netapps.h +++ b/lib/libnet/netapps.h @@ -18,7 +18,8 @@ struct filename_ip; -extern int netboot(int argc, char *argv[]); +extern int netload(char *buffer, int len, char *ret_buffer, int huge_load, + int block_size, char *args_fs, int alen); extern int netsave(int argc, char *argv[]); extern int ping(char *args_fs, int alen); extern int dhcp(char *ret_buffer, struct filename_ip *fn_ip, diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c index 6429405..848609d 100644 --- a/lib/libnet/netload.c +++ b/lib/libnet/netload.c @@ -386,14 +386,11 @@ static void seed_rng(uint8_t mac[]) srand(seed); } -int -netboot(int argc, char *argv[]) +int netload(char *buffer, int len, char *ret_buffer, int huge_load, + int block_size, char *args_fs, int alen) { char buf[256]; int rc; - int len = strtol(argv[2], 0, 16); - char *buffer = (char *) strtol(argv[1], 0, 16); - char *ret_buffer = (char *) strtol(argv[3], 0, 16); filename_ip_t fn_ip; int fd_device; tftp_err_t tftp_err; @@ -403,8 +400,6 @@ netboot(int argc, char *argv[]) 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; - int huge_load = strtol(argv[4], 0, 10); - int32_t block_size = strtol(argv[5], 0, 10); uint8_t own_mac[6]; puts("\n Initializing NIC"); @@ -458,8 +453,16 @@ netboot(int argc, char *argv[]) seed_rng(own_mac); - if (argc > 6) { - parse_args(argv[6], &obp_tftp_args); + if (alen > 0) { + char args[256]; + if (alen > sizeof(args) - 1) { + puts("ERROR: Parameter string is too long."); + return -7; + } + /* Convert forth string into NUL-terminated C-string */ + strncpy(args, args_fs, alen); + args[alen] = 0; + parse_args(args, &obp_tftp_args); if(obp_tftp_args.bootp_retries - rc < DEFAULT_BOOT_RETRIES) obp_tftp_args.bootp_retries = DEFAULT_BOOT_RETRIES; else diff --git a/slof/fs/packages/obp-tftp.fs b/slof/fs/packages/obp-tftp.fs index 30070a6..2e6bcfc 100644 --- a/slof/fs/packages/obp-tftp.fs +++ b/slof/fs/packages/obp-tftp.fs @@ -33,20 +33,13 @@ INSTANCE VARIABLE ciregs-buffer my-parent ihandle>phandle node>path encode-string s" bootpath" set-chosen - \ Generate arg string for snk like - \ "netboot load-addr length filename" - (u.) s" netboot " 2swap $cat s" 60000000 " $cat + 60000000 ( addr maxlen ) \ Allocate 1720 bytes to store the BOOTP-REPLY packet - 6B8 alloc-mem dup >r (u.) $cat - huge-tftp-load @ IF s" 1 " ELSE s" 0 " THEN $cat - \ Add desired TFTP-Blocksize as additional argument - s" 1432 " $cat + 6B8 alloc-mem dup >r ( addr maxlen replybuf ) + huge-tftp-load @ d# 1428 ( addr maxlen replybuf hugetftp blocksize ) \ Add OBP-TFTP Bootstring argument, e.g. "10.128.0.1,bootrom.bin,10.128.40.1" - my-args $cat - \ Zero-terminate string - s" " $cat 2dup + 1 - 0 swap c! - + my-args net-load dup 0< IF drop 0 THEN \ Restore to old client interface register
There is no need anymore to pass most of the arguments as strings, we can use integers and pointers now instead. Signed-off-by: Thomas Huth <thuth@redhat.com> --- lib/libnet/libnet.code | 23 +++++++++-------------- lib/libnet/netapps.h | 3 ++- lib/libnet/netload.c | 21 ++++++++++++--------- slof/fs/packages/obp-tftp.fs | 15 ++++----------- 4 files changed, 27 insertions(+), 35 deletions(-)