Re: Some libl4 patches

[Marcus Brinkmann]

Hi,

I'll beat Neal to the punch.

At Thu, 06 Jan 2005 00:51:29 +0100,
Matthieu Lemerre <address@hidden> wrote:
> I had to make some patches to libl4 to get string message passing works:

Great, thanks a lot.  We will guide you through the usual peer review... ;)
 
> in ia32/vregs.h: There was some problem with Buffer register
> loading/storing (basically, it didn't worked because the data were
> processed backward)

Yes.  The same problem exists in the powerpc port.
 
> 
> in ipc.h:
> 
> functions _L4_msg_buffer_append_simple_rcv_string and
> _L4_msg_buffer_append_rcv_string:
> 
> There was a single | instead of ||, and , as || is yet used in
> _L4_accept_strings, I put || to normalize.

Ok.  There is no functional change by this, but it gives more
information to the compiler if we use ||.
 
> They also did not work because of a do ... while done instead of a
> while: the first bytes of the message buffer were never used.
>
> So I propose the folowing: a call to l4_msg_buffer_clear would set
> both the first and the second word to 0, and we test whether the
> second word is set or not for knowing whether or not there is
> already a string item in the message buffer (because putting one
> would set this second word to the address of the string or substring
> ptr, thus overriding the 0)
> 
> However, there is a problem if we can use the null pointer as the send
> or receive address of a string.

Indeed, so your fix is invalid.  0 is a perfectly fine address for a
string buffer, as far as L4 is concerned.  There is another

So, you would have to check the first word of the string item.  This
is still flawed, but in a more subtle way, as even a string buffer of
length 0 is valid!  And thus you could not differentiate between no
string buffer items (buffer[0] == 0) and a single simple string buffer
item of length 0.  What the official L4 library does is to set
buffer[0] to 0 and buffer[2] to 0, and to set buffer[2] to ~0 if the
first string item has been set.  The trick works because either there
is no string item, then both are 0, or there is a single simple string
item, then buffer[2] is 0, or there is a single compound string item,
then buffer[0] is != 0 because of the "nr_substrings" field, or there
is more than one string item, and then buffer[0] is != 0 because of
the "cont" field.

I think this trick is basically the only way to allow for all possible
border cases, so we have to adopt it.  The algorithm is then:

clear: buffer[0] = buffer[2] = 0;

append: if (buffer[0] == 0 && buffer[2] == 0)
          There is no string item yet.
          Use first one.
          if (this is simple string item)
            buffer[2] = ~0;
          else
            Do nothing - buffer[0] is already guaranteed to be != 0.
        else
          Walk the string items until we find the buffer word after
          the last one.
          Use it, set previous cont field to 1.
          At least now buffer[0] will be != 0 (if this was the second added).
 
> I didn't checked, but I wonder if _L4_add_substring_to hasn't the same
> problem.

Could very well be.  Could you please check?

It would be very worthwhile to write test cases for all these
functions, that test for the silly border cases like adding a single
empty string item etc.
 
> I also wonder if we shouldn't add a l4_string_item "creator" function
> in the GNU interface to libl4; this function is useful (at least, it
> was useful for me).

Is this what you wrote about in your other mail?  I think I saw a mai
by you that I marked in my inbox and didn't reply to yet.

I'd like you to make a new patch which we can apply that includes a
fix for the powerpc port, addresses the minor nitpicks from Neal,
implements the above solution and also fixes the other problematic
functions which have the same problems.  Can you please do that?
Otherwise it is already great that you found the problem, and fixing
it wouldn't be too hard for me either, so just let us know if you want
to keep going on it.  We certainly appreciate it.

> PS: I received the legal papers, I will send them back tomorrow.

Great!

Thanks a lot,
Marcus