Re: [Tsp-devel] BB: new dump & load features

[Eric Noulard]

2007/5/4, Frederik Deweerdt <address@hidden>:
> On Fri, May 04, 2007 at 05:16:02PM +0200, Eric Noulard wrote:
> > Does load use the sum before loading?
> Nope, it would be a nice feature though. But for that we need another
> way to parse the options in the bbtools (ie. depending on the command
> itself). So I delayed that part.

I think you may already offer the feature using the following
positional options handling:

bb_tools load thebb bbdump.xml [check]

this is the way "bb_tools bb_publish" work:

Usage : bb_publish <bbname> <symbol_name> [<symbol_type>=UINT32] [<arraysize>=1]

using this scheme is handy as long as you don't want to specify
multiple options in different order.

> > How bb_tools is supposed to use the md5sum?
>
> At the moment, script around the bb_tools call will do the check (for us).
> In the future, we should have a "bb_tools load -c my.xml" to enforce
> the check before loading.

Yes it is a good thing to offers this.
I think the "bb_load" wrapper may easily do the job.
Moreover I think I'd like to be able to tell bb_dump if I want the
firstline checksum or not.
As a possibility I would rather have:

bb_dump <bbname> xml                --> xml output NO checksum
bb_dump <bbname> xml check      --> xml output WITH checksum
bb_dump <bbname> [classic]         --> classic output NO checksum
bb_dump <bbname> classic check  ...

and for bb_load/ bb_tools load

bb_load <bbname> <dumpfile> [check]

would load the file with or without checksum check.

I'd like to have symmetric behavior for dump/load.
And in fact I would rather have

bb_dump / bb_restore
bb_save   / bb_load

even if bb_dump == bb_save and bb_load == bb_restore.

> > >Note that you need openssl libraries for the md5 sum, and libxml2 for
> > >the xml format.
> > >Ideas and reports of success or failure are welcomed :)

And now comes perhaps more design choices remarks regarding
integrity checksum.

0) What are you design needs for the checksum/hash?
   Security?
   Integrity?
   ...??
   My following remarks may be stupid depending on your needs.

1) I think I would rather go for SHA-1 hashing instead of MD5
   for differents reason:
       1.a) We already have "built-in" SHA-1 function in bb_sha1.[hc]
               which would minimize dependency against third party libs.
               bb_sha1 is used for generating [quasi]-unique IPC key
               see bb_check_id
        1.b) Moreover SHA-1 is known to be more secure than MD5
               in the sense that it is "easier" to fing collision
               with MD5 than it is with SHA-1.
                 http://en.wikipedia.org/wiki/MD5
                 http://en.wikipedia.org/wiki/SHA-1
              or dicussion here:

http://www.schneier.com/blog/archives/2005/11/nist_hash_works_4.html
       1.c) There is a sha1sum(1) command just as the md5sum(1) one

2) I'd rather keep the checksuming code outside the BB code.
   I think it would be more flexible to do checksum in a scripts around
   the bb_tools as discussed ealier since it will be easier to change
   algorithm/command with ones available on the concern host.
   (md5sum, sha1sum, cksum, etc...)

   A one liner may be:
   bb_dump bb_simu xml | sha1sum
   whereas today I have to go with:
   bb_dump bb_simu xml | awk '{if (NR >1) { print $0 }}' | sha1sum

   I think it is better to keep checksum clearly separated from dump
   because:
         2.a) Checksum is of no use in the "classic" dump format
             because this format display addresses which are not "constant"
            (try several calls to
              bb_dump bb_simu classic | awk '{if (NR >1) { print $0
}}' | md5sum
            )

          2.b) Because current bb_tools/bb_core did not access files
              (besides stdin, stdout, stderr) and I think it is a valuable
              property. The current implementation of md5sum
              use tmpfile(3) call.
              (I think that bb_load may well be feed its input from stdin)

         2.c) It would be easier to chose/change hash algorithm

         2.d) It will minimize the dependency of BB from third party libs
                like OpenSSL.

         2.e) BB is a relatively low-level / low resource consuming tool
                hash algorithms may cost you more than expected.

3) If you ever want to keep the checksum code inside BB code
   I think I would rather put the checksum line at the end of the stream.
   This way you can get rid of the temp file and put the checksuming
   code from within the printer (2nd argument of  bb_dump API)
   and do checksuming while printing.

4) Some source code design choice remark

  *  I think that process_node should call bb_value_direct_write
      in order to avoid code duplication.
      bb_value_direct_write implements almost the same switch(type),
      we will have to check the CHAR/UCHAR and UserType cases though.

  * XML related code may be put in bb_xml.[ch]

     load_xml_file may be splitted into
              bb_xml_load(xmlTextReaderPtr reader,  volatile S_BB_T **bb)
              bbtools_load_xml(bbtools_request_t* req)
              (bb_xml_load going into bb_xml.c
               bbtools_load_xml going into bbtools.c will call bb_xml_load)

     and
     process_node to
     bb_xml_process_node(xmlTextReaderPtr reader,  volatile S_BB_T
**bb, const char* bbname)

Whatever all my possibly silly or weird remarks,
from my point of view this is a very interesting feature for the Blackboard
to have such save/load, dump/restore feature, this will be interesting for
people wanting to use (or currently using) a Blackboard in their simulator
they may find here a nice building block for save_context / restore_context
kind of feature we usually find in those environment.

Thank you very much for this feature.

As usual I may put my hand on the keyboard to implement
part or all the changes I suggest  but before I do that I'd like
to have your (all of you) point of view.

--
Erk