Re: using large (>2G) files with clisp

[Bruno H. <br...@cl...>]

Don Cohen wrote:
>  > > - I notice:
>  > > #ifdef WIN32_NATIVE
>  > >   #define handle_lseek(stream,handle,offset,mode,result_assignment)
>  > >   { var DWORD result = SetFilePointer(TheHandle(handle),offset,
>  > > NULL,mode); This DWORD (and similar things) should also be size_t ?
>  >
>  > No. Windows uses the type DWORD for this, so we use the type DWORD as
>  > well. But you change the next line
>  >
>  >           (result_assignment result);
>  > to
>  >           (result_assignment (off_t)result);
>
> This is strange.  If off_t is defined for MS at all then certainly it
> should be the result of SetFilePointer, shouldn't it?
> And if it's not defined, then we should define it so as to be
> compatible with the rest of the code.

Windows has off_t, typedef'ed to 'long', but SetFilePointer returns DWORD.
It's not logical, I know. But that's the way it is. And I prefer to use
the system types in system calls rather than the types that I would
find better, because sometimes the values are passed or returned by reference,
not by return value, and then it matters whether you write 'DWORD' or
'size_t'.

In order to stop this lengthy discussion, I've committed a patch that
implements the use of off_t.

>  > No, byte counts (or anything that refers to a number of bytes in memory)
>  > remains size_t.
>
> This is a bit confusing cause the current code does not use size_t for
> byte counts.  It seems to me that anything that will be used as a
> number of bytes to read or write must be size_t, but (as above) that's
> not necessarily the same as the size of something in memory.

Yes, for a perfect migration to 64-bit world, clisp should probably use
size_t, with an intermediate typedef (in order to catch all platforms
which might get size_t wrong - remembering that Windows gets wchar_t wrong,
I wouldn't be surprise if Windows-64-bit got size_t wrong).

> Why not just define size_t, etc. appropriately for MS?

Becayse Windows already has size_t. We can choose to use it or not to use it,
but we cannot redefine it.

>  > But you have to be careful when you combine an off_t and a size_t. The
>  > types in an expression like
>  >              position += bytecount;
>  > are not the same any more. On the LHS you have a possibly 64-bit off_t.
>
> I don't see the problem.  I'd expect that off_t >= size_t so you could
> add a size_t to an off_t.

The problem comes when you do something like

                 position += bytecount1 - bytecount2;

and bytecount1 and bytecount2 are size_t, but position is off_t. Fortunately
such code doesn't exist in stream.d.

>   var uintB* startptr = &TheSbvector(*bytearray_)->data[start];
>   var uintB* endptr =
>     UnbufferedStreamLow_read_array(stream)(stream,startptr,len,no_hang);
> (which calls read_helper on its arguments)
> So yes, start is the array offset rather than the file offset.
> On the other hand startptr above is a file offset so should be off_t.

????

> I still think that len in all cases should be size_t.

len is described as "length of byte sequence to be filled" must be
size_t (or a usually equivalent typedef), not off_t.

Bruno