Re: [Ocaml-lib-devel] IO update (2)

[Nicolas C. <war...@fr...>]

> > Here's another IO update :
>
> Just some remarks:

They're actualy quite interesting ones :)

> o Wouldn't it be better to rename read_i32 and write_i32 to
>   read_i31 and write_i31 ? Then you could add real read_i32 and
>   write_i32 functions based on native ints.

The values that can be readed/written are 31 bits limited caml integers (on
32 bits platform , since 64 bits have 63 bits integers). But the size of the
data readed / written is exactly 32 bits. That's true that some people might
need the read_i32_full / write_i32_full functions that are returning int32
values, so let's add them. But having functions which name are claming
reading/writting 31 bits looks highly suspicious for people who does not
know about ocaml implementation details :-)

> o read_line and read_string do not handle the case that the input
>   becomes empty before the terminating character is read.

That's true.
I just fixed it now - in the read_line case, thanks for the report. I will
not in the read_string case since read_string is working on binary files and
an not-null terminated string might be considered as the file being cut.

> o As already pointed out by someone else, using \0 as terminating
>   character for strings is questionable. To support strings containing
>   \0 one could store strings in the format
>
>     "number of characters" + "data".

I already answered about this : I have been written the addons to IO in
order to work easily when using C styled genered files. In C, strings are
null terminated.

> o A slight optimisation of write_byte would be to use
>   unsafe_char_of_int.

I'll have a look at that.

> o What about read_utf8 and write_utf8 ?

I don't have knowledge in internationalizion, if you have some ideas about
this, please feel free to contribute !

> o Have you made up your mind about supporting seekable streams?

Not yet. This would need to add another closure to the IO prototype : I'm
not yet sure it's worth it.

Regards,
Nicolas Cannasse