Good Morning,
I am working on some primitives to allow to populate a SQFile* based on int fd/FILE* and tried to understand the semantics of isStdioStream. When opening a non-file (e.g. a file descriptor returned from inotify_init1) I noticed this gem:
bytesRead = 0; do { clearerr(file); if (fread(dst, 1, 1, file) == 1) { bytesRead += 1; if (dst[bytesRead-1] == '\n' || dst[bytesRead-1] == '\r') break; } } while (bytesRead <= 0 && ferror(file) && errno == EINTR);
* >>#primRead:into:startingAt:count: will ignore count and only fill one byte at max. After the first successful fread the while loop will be left. The if for checking for a \n or \r could be deleted. I assume the intention was to read until bytesRead == count or \n\r? So e.g. even count: 0 will be ignored and turned into a '1'.
* To me line buffering looks a bit like a workaround but if we want this semantic for stdio can't we use fgets?
regards holger
Hi Holger,
On Fri, Sep 23, 2016 at 12:33 AM, Holger Freyther holger@freyther.de wrote:
Good Morning,
I am working on some primitives to allow to populate a SQFile* based on int fd/FILE* and tried to understand the semantics of isStdioStream. When opening a non-file (e.g. a file descriptor returned from inotify_init1) I noticed this gem:
bytesRead = 0; do { clearerr(file); if (fread(dst, 1, 1, file) == 1) { bytesRead += 1; if (dst[bytesRead-1] == '\n' || dst[bytesRead-1] == '\r') break; } } while (bytesRead <= 0 && ferror(file) && errno == EINTR);
#primRead:into:startingAt:count: will ignore count and only fill onebyte at max. After the first successful fread the while loop will be left. The if for checking for a \n or \r could be deleted. I assume the intention was to read until bytesRead == count or \n\r? So e.g. even count: 0 will be ignored and turned into a '1'.
Right. I added code to make reads from stdin work. I was implementing the following arm for stdin:
else do { clearerr(file); bytesRead = fread(dst, 1, count, file); } while (bytesRead <= 0 && ferror(file) && errno == EINTR);
i.e. the preexisting code insisted on only reading one character at a time.
And as the comment says:
/* Line buffering in fread can't be relied upon, at least on Mac OS X * and mingw win32. So do it the hard way. */ bytesRead = 0; do { clearerr(file); if (fread(dst, 1, 1, file) == 1) { bytesRead += 1; if (dst[bytesRead-1] == '\n' || dst[bytesRead-1] == '\r') break; } } while (bytesRead <= 0 && ferror(file) && errno == EINTR);
- To me line buffering looks a bit like a workaround but if we want this
semantic for stdio can't we use fgets?
That seems like a good idea. So you're proposing
fgets(dst,1,file)
right? Much nicer. To whoever implements this please test on Windows and Mac OS and Linux.
regards holger
_,,,^..^,,,_ best, Eliot
On 23 Sep 2016, at 19:22, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Holger,
Hey!
And as the comment says:
/* Line buffering in fread can't be relied upon, at least on Mac OS X * and mingw win32. So do it the hard way. */ bytesRead = 0; do { clearerr(file); if (fread(dst, 1, 1, file) == 1) { bytesRead += 1;
bytesRead > 0 now
if (dst[bytesRead-1] == '\n' || dst[bytesRead-1] == '\r') break; } } while (bytesRead <= 0 && ferror(file) && errno == EINTR);
^^^ false && ? && ?
That seems like a good idea. So you're proposing
fgets(dst,1,file)
right? Much nicer. To whoever implements this please test on Windows and Mac OS and Linux.
I had fgets(dst, count, file) in mind but after re-reading the manpage we don't need/want the NUL termination here. (and I don't think doing something like char buf[count+1]; fgets.; memcpy(dst, buf, size);
My new proposal then is to change
while (bytesRead <= 0 && ferror(file) && errno == EINTR)
to:
while (bytesRead <= count && ferror(file) && errno == EINTR)
cheers holger
So if I understand it, isStdioStream means one of stdin stdout stderr? It would deserve a comment because it's not that obvious, to me it means level 3 standard input/output like in man 3 stdio, as opposed to level 2 file descriptors. Not just the pre-opened input/output/error streams.
2016-09-23 21:08 GMT+02:00 Holger Freyther holger@freyther.de:
On 23 Sep 2016, at 19:22, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Holger,
Hey!
And as the comment says:
/* Line buffering in fread can't be relied upon, at least on Mac
OS X
* and mingw win32. So do it the hard way. */ bytesRead = 0; do { clearerr(file); if (fread(dst, 1, 1, file) == 1) { bytesRead += 1;
bytesRead > 0 now
if (dst[bytesRead-1] == '\n' || dst[bytesRead-1] == '\r') break; } } while (bytesRead <= 0 && ferror(file) && errno == EINTR);
^^^ false && ? && ?
That seems like a good idea. So you're proposing
fgets(dst,1,file)
right? Much nicer. To whoever implements this please test on Windows
and Mac OS and Linux.
I had fgets(dst, count, file) in mind but after re-reading the manpage we don't need/want the NUL termination here. (and I don't think doing something like char buf[count+1]; fgets.; memcpy(dst, buf, size);
My new proposal then is to change
while (bytesRead <= 0 && ferror(file) && errno == EINTR)
to:
while (bytesRead <= count && ferror(file) && errno == EINTR)
cheers holger
On Fri, Sep 23, 2016 at 12:08 PM, Holger Freyther holger@freyther.de wrote:
On 23 Sep 2016, at 19:22, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Holger,
Hey!
And as the comment says:
/* Line buffering in fread can't be relied upon, at least on Mac
OS X
* and mingw win32. So do it the hard way. */ bytesRead = 0; do { clearerr(file); if (fread(dst, 1, 1, file) == 1) { bytesRead += 1;
bytesRead > 0 now
if (dst[bytesRead-1] == '\n' || dst[bytesRead-1] == '\r') break; } } while (bytesRead <= 0 && ferror(file) && errno == EINTR);
^^^ false && ? && ?
While we haven't read anything and there's an error and the error is due to being interrupted. So exit if we've read something (because we're done), or if we haven't and there was no error (because there was no input) or we haven't and there was an error other than being interrupted (because something wen't wrong and looping won't fix it)
So the condition under which to keep trying is having yet to read input and having been interrupted. In all other circumstances we should exit. Doesn't that make sense?
That seems like a good idea. So you're proposing
fgets(dst,1,file)
right? Much nicer. To whoever implements this please test on Windows
and Mac OS and Linux.
I had fgets(dst, count, file) in mind but after re-reading the manpage we don't need/want the NUL termination here. (and I don't think doing something like char buf[count+1]; fgets.; memcpy(dst, buf, size);
My new proposal then is to change
while (bytesRead <= 0 && ferror(file) && errno == EINTR)
to:
while (bytesRead <= count && ferror(file) && errno == EINTR)
cheers holger
But what happens if we read, say, half the requested input and then there's an error. The VM shouldn't lock spinning trying to read input. For example, what if the image requests reading 100 characters and t6he user types end-of-file as input? We don't want to spin trying to read input which will never become available.
_,,,^..^,,,_ best, Eliot
On 24 Sep 2016, at 00:05, Eliot Miranda eliot.miranda@gmail.com wrote:
Good Morning!
On Fri, Sep 23, 2016 at 12:08 PM, Holger Freyther holger@freyther.de wrote:
while (bytesRead <= 0 && ferror(file) && errno == EINTR);
^^^ false && ? && ?
While we haven't read anything and there's an error and the error is due to being interrupted. So exit if we've read something (because we're done), or if we haven't and there was no error (because there was no input) or we haven't and there was an error other than being interrupted (because something wen't wrong and looping won't fix it)
Please remember that this is the stdin case:
a.) If on each primitive call we want to read 0 or 1 character let's remove the check if a \r or \n was read (as bytesRead == 1 and the loop will be terminated anyway).
b.) Let's try to read closer to count? If we assume line buffering and we have read one character it is likely we can read another until either count or \r\n is reached?
So the condition under which to keep trying is having yet to read input and having been interrupted. In all other circumstances we should exit. Doesn't that make sense?
true, boolean logic is hard ;)
while (continueToRead(file, count, bytesRead))
static inline bool continueToRead(FILE* file, size_t count, size_t bytesRead) { if (ferror(file) && errno != EINTR) return 0; return bytesRead < count; }
But what happens if we read, say, half the requested input and then there's an error. The VM shouldn't lock spinning trying to read input. For example, what if the image requests reading 100 characters and t6he user types end-of-file as input? We don't want to spin trying to read input which will never become available.
right on first error we should bail out, I think the above is doing that?
holger
On Sat, Sep 24, 2016 at 02:37:38PM +0200, Holger Freyther wrote:
Please remember that this is the stdin case:
a.) If on each primitive call we want to read 0 or 1 character let's remove the check if a \r or \n was read (as bytesRead == 1 and the loop will be terminated anyway).
b.) Let's try to read closer to count? If we assume line buffering and we have read one character it is likely we can read another until either count or \r\n is reached?
What were the original issues that motivated different handling of stdio streams in the sqFileReadIntoAt function? Based on the change history, I assume that it was driven by platform specific issues on Windows or Mac, but I don't know what the actual issues were.
My assumption is that isStdioStream was originally added to SQFile by Andreas to allow support for win32 console behavior. The implementation of sqFileReadIntoAt for Windows is in the win32 tree (it uses HANDLE rether than (FILE *) and is different from the Cross implementation used by other platforms). So the conditional code in Cross is presumably related to Mac or OS X issues, is that right?
Curious,
Dave
vm-dev@lists.squeakfoundation.org