| Message ID | 1465444006.2948.24.camel@kernel.crashing.org |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jun 09, 2016 at 01:46:46PM +1000, Benjamin Herrenschmidt wrote: > +: do-dir ( devstr len -- ) > + cr ." Directory of: " 2dup type ." ... " That is debug code, right? The user knows what he typed, and if not, he can always look one line up ;-) > + open-dev dup IF > + s" dir" 2 pick ['] $call-method CATCH IF > + ." no dir method on target !" cr > + 3drop > + THEN > + close-dev cr > + ELSE drop THEN > +; dup IF ... ELSE drop THEN is the same as ?dup IF ... THEN > +: dir ( "{devstring}" -- ) > + parse-word de-alias do-dir > +; A more usual name for do-dir would be (dir) or $dir . > +: find-path ( dir-cluster name len -- cluster file-len is-dir? true | false ) Things will be simpler if you store to dir? directly here, instead of passing it around. As an aside, a word that ends in a question mark usually leaves a boolean on the stack, not an address. > + \ Store right side on return stack > + 2>r Useless comments are useless ;-) Segher
On Thu, 2016-06-09 at 02:18 -0500, Segher Boessenkool wrote: > On Thu, Jun 09, 2016 at 01:46:46PM +1000, Benjamin Herrenschmidt > wrote: > > > > +: do-dir ( devstr len -- ) > > + cr ." Directory of: " 2dup type ." ... " > That is debug code, right? The user knows what he typed, and if not, > he can always look one line up ;-) Well, it's de-aliased which is handy, that's why I left it there. > > > > + open-dev dup IF > > + s" dir" 2 pick ['] $call-method CATCH IF > > + ." no dir method on target !" cr > > + 3drop > > + THEN > > + close-dev cr > > + ELSE drop THEN > > +; > dup IF ... ELSE drop THEN > > is the same as > > ?dup IF ... THEN ok. > > > > +: dir ( "{devstring}" -- ) > > + parse-word de-alias do-dir > > +; > A more usual name for do-dir would be (dir) or $dir . ok. > > > > +: find-path ( dir-cluster name len -- cluster file-len is-dir? > > true | false ) > Things will be simpler if you store to dir? directly here, instead of > passing it around. Not necessarily and I don't like functions like that having side effects, it makes the code even more convoluted. This find-path has no side effect, only inputs and outputs. > As an aside, a word that ends in a question mark usually leaves a > boolean on the stack, not an address. ok. > > > > + \ Store right side on return stack > > + 2>r > Useless comments are useless ;-) It's not useless, it wasn't clear to me which side was left and which one was right :-) Cheers, Ben. > Segher
On Thu, Jun 09, 2016 at 06:07:12PM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2016-06-09 at 02:18 -0500, Segher Boessenkool wrote: > > On Thu, Jun 09, 2016 at 01:46:46PM +1000, Benjamin Herrenschmidt > > wrote: > > > > > > +: do-dir ( devstr len -- ) > > > + cr ." Directory of: " 2dup type ." ... " > > That is debug code, right? The user knows what he typed, and if not, > > he can always look one line up ;-) > > Well, it's de-aliased which is handy, that's why I left it there. Ah yes. > > > +: find-path ( dir-cluster name len -- cluster file-len is-dir? > > > true | false ) > > Things will be simpler if you store to dir? directly here, instead of > > passing it around. > > Not necessarily and I don't like functions like that having side > effects, it makes the code even more convoluted. This find-path > has no side effect, only inputs and outputs. Yes, ten gazillion inputs and outputs now ;-) You already store that state slightly later, why not do it here already and have simpler code, much easier to read? > > > + 2>r > > Useless comments are useless ;-) > > It's not useless, it wasn't clear to me which side was left and which > one was right :-) left is left, right is right -- very handy semantics :-) If a word is hard to follow try factoring it. Stack comments inside a word are almost always a bad idea. Segher
On Thu, 2016-06-09 at 03:27 -0500, Segher Boessenkool wrote: > > Yes, ten gazillion inputs and outputs now ;-) > > You already store that state slightly later, why not do it here already > and have simpler code, much easier to read? I tried, I actually had it that way... not that much easier and I'm paranoid. If I want to come back in 2 years and use find-path for something else, and forget it clobbers the global is-dir ... I really really hate side effects :) > If a word is hard to follow try factoring it. Stack comments inside > a word are almost always a bad idea. I like how the firmworks code is done, they always have a right column with the stack state ;-) Cheers, Ben.
On Thu, Jun 09, 2016 at 10:13:41PM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2016-06-09 at 03:27 -0500, Segher Boessenkool wrote: > > > > Yes, ten gazillion inputs and outputs now ;-) > > > > You already store that state slightly later, why not do it here already > > and have simpler code, much easier to read? > > I tried, I actually had it that way... not that much easier and I'm > paranoid. If I want to come back in 2 years and use find-path for > something else, and forget it clobbers the global is-dir ... I really > really hate side effects :) But *most* things do change state, already. What is on the *stack* is state. Putting things in a locally-used variable is not so bad. Anyway, your choice :-) Segher
diff --git a/slof/fs/boot.fs b/slof/fs/boot.fs index e99a164..e436380 100644 --- a/slof/fs/boot.fs +++ b/slof/fs/boot.fs @@ -174,6 +174,21 @@ defer go ( -- ) \ Generic device load method: \ * +: do-dir ( devstr len -- ) + cr ." Directory of: " 2dup type ." ... " + open-dev dup IF + s" dir" 2 pick ['] $call-method CATCH IF + ." no dir method on target !" cr + 3drop + THEN + close-dev cr + ELSE drop THEN +; + +: dir ( "{devstring}" -- ) + parse-word de-alias do-dir +; + : do-load ( devstr len -- img-size ) \ Device method wrapper use-load-watchdog? IF \ Set watchdog timer to 10 minutes, multiply with 2 because DHCP diff --git a/slof/fs/packages/fat-files.fs b/slof/fs/packages/fat-files.fs index 5d578f1..ac2f141 100644 --- a/slof/fs/packages/fat-files.fs +++ b/slof/fs/packages/fat-files.fs @@ -33,6 +33,8 @@ INSTANCE VARIABLE root-offset INSTANCE VARIABLE cluster-offset INSTANCE VARIABLE #clusters +INSTANCE VARIABLE dir? + : seek s" seek" $call-parent ; : read s" read" $call-parent ; @@ -54,7 +56,7 @@ CREATE fat-buf 8 allot fat-buf 8 read 8 <> ABORT" fat-files read-fat: read failed" fat-buf 8c@ bxjoin fat-type @ dup >r 2* #split drop r> #split rot IF swap THEN drop ; - + INSTANCE VARIABLE next-cluster : read-cluster ( cluster# -- ) @@ -130,14 +132,35 @@ CREATE dos-name b allot : find-file ( dir-cluster name len -- cluster file-len is-dir? true | false ) make-dos-name read-dir BEGIN (find-file) 0= WHILE next-cluster @ WHILE next-cluster @ read-cluster REPEAT false ELSE true THEN ; -: find-path ( dir-cluster name len -- cluster file-len true | false ) - dup 0= IF 3drop false ." empty name " EXIT THEN - over c@ [char] \ = IF 1 /string RECURSE EXIT THEN - [char] \ split 2>r find-file 0= IF 2r> 2drop false ." not found " EXIT THEN - r@ 0<> <> IF 2drop 2r> 2drop false ." no dir<->file match " EXIT THEN - r@ 0<> IF drop 2r> RECURSE EXIT THEN - 2r> 2drop true ; - + +: find-path ( dir-cluster name len -- cluster file-len is-dir? true | false ) + dup 0= IF + \ empty name, assume directory + 2drop 0 true true EXIT + THEN + \ Strip leading backslashes + over c@ [char] \ = IF + 1 /string RECURSE EXIT + THEN + \ Split at backslash + [char] \ split + \ Store right side on return stack + 2>r + find-file + 0= IF + 2r> 2drop false ." not found " EXIT + THEN + \ right side (from stack) has non-0 len, must be a dir + dup 0= r@ 0<> and IF + 3drop 2r> 2drop false ." path component not a dir " EXIT + THEN + r@ 0<> IF + 2drop 2r> RECURSE EXIT + THEN + 2r> 2drop + true +; + : do-super ( -- ) 0 200 read-data data @ 0b + 2c@ bwjoin bytes/sector ! @@ -204,7 +227,18 @@ INSTANCE VARIABLE pos-in-data file-len @ read dup file-len @ <> ABORT" fat-files: failed loading file" ; : close free-data ; + +: dir + dir? @ IF file-cluster @ .dir ELSE ." not a directory!" cr THEN + ; + : open do-super - 0 my-args find-path 0= IF close false EXIT THEN - file-len ! file-cluster ! 0 0 seek 0= ; + 0 my-args find-path + 0= IF free-data false EXIT + THEN + dir? ! file-len ! file-cluster ! + dir? @ IF + 0 0 seek 0= + ELSE true THEN +;
This adds a method akin to "boot" and "load" which takes the subsequent command line arguments, parses them as a device specification and calls the dir method on said device Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- slof/fs/boot.fs | 15 ++++++++++++ slof/fs/packages/fat-files.fs | 56 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 11 deletions(-)