[libbluray-devel] cast file read/write result to size_t

[Petri Hintukainen]

On pe, 2013-04-12 at 00:26 +0200, Joakim Plate wrote:
> This will break error checking. They can return negative. You could do
> ssize_t perhaps.

It wouldn't make any difference, return values are stored to size_t
everywhere. Also, negative values would cause problems in bit reader.

I think this API is not well designed. At least it should be
documented...

Negative values are not needed and shouldn't be allowed at all:
  1) It does not make any sense to read/write zero or negative
     amount of bytes.
  2) If request size is unsigned also result must be unsigned
     (range of result type can't be smaller than range of
     request type)
  3) Returning anything else than requested already indicates
     failure (EOF or error). There's no need for any special
     error code.

Current implementation expects fread/fwrite semantics:
Return value is always stored in (unsigned) size_t. The range of the
type is irrelevant, only small chunks are read (up to 64k). Every place
where file_read() is used the code checks if return value matches
requested value. The only exception is bit reader: it stores the return
value directly as size of chunk. (it will fail with negative values).

[file_write() is useless, BD filesystem is always read-only.]

As current code already assumes return values between 0 and size
requested, maybe those function signatures could be changed to use
uint64_t instead of int64_t ?
Technically it is API change but it does not change ABI or
functionality, as the current implementation already excepts those
values to be non-negative.
 
> On Thu, Apr 11, 2013 at 11:30 PM, Ian Curtis <git at videolan.org> wrote:
>         libbluray | branch: master | Ian Curtis <i.curtis at gmail.com> |
>         Fri Apr 12 00:24:29 2013 +0300|
>         [a8b18b411f25ff55f40cc6cb2517efada2a6a9d3] | committer: hpi1
>         
>         cast file read/write result to size_t
>         
>         >
>         http://git.videolan.org/gitweb.cgi/libbluray.git/?a=commit;h=a8b18b411f25ff55f40cc6cb2517efada2a6a9d3
>         ---
>         
>          src/file/file.h |    4 ++--
>          1 file changed, 2 insertions(+), 2 deletions(-)
>         
>         diff --git a/src/file/file.h b/src/file/file.h
>         index cad5d54..080485b 100644
>         --- a/src/file/file.h
>         +++ b/src/file/file.h
>         @@ -39,8 +39,8 @@
>          #define file_seek(X,Y,Z) X->seek(X,Y,Z)
>          #define file_tell(X) X->tell(X)
>          #define file_eof(X) X->eof(X)
>         -#define file_read(X,Y,Z) X->read(X,Y,Z)
>         -#define file_write(X,Y,Z) X->write(X,Y,Z)
>         +#define file_read(X,Y,Z) (size_t)X->read(X,Y,Z)
>         +#define file_write(X,Y,Z) (size_t)X->write(X,Y,Z)
>         
>          BD_PRIVATE extern BD_FILE_H* (*file_open)(const char*
>         filename, const char *mode);
>         
>         
>         _______________________________________________
>         libbluray-devel mailing list
>         libbluray-devel at videolan.org
>         http://mailman.videolan.org/listinfo/libbluray-devel
> 
> _______________________________________________
> libbluray-devel mailing list
> libbluray-devel at videolan.org
> http://mailman.videolan.org/listinfo/libbluray-devel