This moves the free() call for the file buffer before the malloc() that occurs when closing a temp file, which should at least slightly reduce the chances that the malloc() call fails.
Previously, the code for closing a temporary file assumed that malloc would succeed. If it did not, the code would trash memory and (at least in my testing) crash the system. Now it checks for and handles malloc failures, although they will still lead to the temporary file not being deleted.
Here is a test program illustrating the problem:
#include <stdio.h>
#include <stdlib.h>
int main(void) {
FILE *f = tmpfile();
if (!f)
return 0;
void *p;
do {
p = malloc(8*1024);
} while (p);
fclose(f);
}
This can happen, e.g., if there is an IO error or if there is insufficient free disk space to flush the data. In this case, fclose should return -1 to report an error, but it should still effectively close the stream and deallocate the buffer for it. (This behavior is explicitly specified in the C99 and later standards.)
Previously, ORCA/C effectively left the stream open in these cases. As a result, the buffer was not deallocated. More importantly, this could cause the program to hang at exit, because the stream would never be removed from the list of open files.
Here is an example program that demonstrates the problem:
/*
* Run this on a volume with less than 1MB of free space, e.g. a floppy.
* The fclose return value should be -1 (EOF), indicating an error, but
* the two RealFreeMem values should be close to each other (indicating
* that the buffer was freed), and the program should not hang on exit.
*/
#include <stdio.h>
#include <stddef.h>
#include <memory.h>
#define BUFFER_SIZE 1000000
int main(void) {
size_t i;
int ret;
printf("At start, RealFreeMem = %lu\n", RealFreeMem());
FILE *f = fopen("testfile", "wb");
if (!f)
return 0;
setvbuf(f, NULL, _IOFBF, BUFFER_SIZE);
for (i = 0; i < BUFFER_SIZE; i++) {
putc('x', f);
}
ret = fclose(f);
printf("fclose return value = %d\n", ret);
printf("At end, RealFreeMem = %lu (should be close to start value)\n",
RealFreeMem());
}
If the format string is empty or contains only %n conversions, then nothing should be read from the stream, so no error should be indicated even if it is at EOF. If a directive does read from the stream and encounter EOF, that will be handled when the directive is processed.
This could cause scanf to pause waiting for input from the console in cases where it should not.
These are similar enough that they can use the same code with just a few conditionals, which saves space.
(This same code can also be used for binary when that is added.)
These print a floating-point number in a hexadecimal format, with several variations based on the conversion specification:
Upper or lower case letters (%A or %a)
Number of digits after decimal point (precision)
Use + sign for positive numbers? (+ flag)
Use leading space for positive numbers (space flag)
Include decimal point when there are no more digits? (# flag)
Pad with leading zeros after 0x? (0 flag)
If no precision is given, enough digits are printed to represent the value exactly. Otherwise, the value is correctly rounded based on the rounding mode.
This is what the standards require. Previously, the '0' flag would effectively override '-'.
Here is a program that demonstrates the problem:
#include <stdio.h>
int main(void) {
printf("|%-020d|\n", 123);
printf("|%0-20d|\n", 123);
printf("|%0*d|\n", -20, 123);
}
ORCA/C's tmpnam() implementation is designed to use prefix 3 if it is defined and the path is sufficiently short. I think it was intended to allow up to a 15-character disk name to be specified, but it used a GS/OS result buffer size of 16, which only leaves 12 characters for the path, including initial and terminal : characters. As such, only up to a 10-character disk name could be used. This patch increases the specified buffer size to 21, allowing for a 17-character path that can encompass a 15-character disk name.
Bugs fixes:
*fgets() would write 2 bytes in the buffer if called with n=1 (should be 1).
*fgets() would write 2 bytes in the buffer if it encountered EOF before reading any characters, but the EOF flag had not previously been set. (It should not modify the buffer in this case.)
*fgets() and gets() would return NULL if EOF was encountered after reading one or more characters. (They should return the buffer pointer).
The __-prefixed versions were introduced for use in <stdio.h> macros that have since been removed, so they are not really necessary any more. However, they may be used in old object files, so those symbols are still included for now.
The error could occur because fseek calls fflush followed by ftell. fflush would reset the file position as if the characters in the putback buffer were removed, but ftell would still see them and try to adjust for them (in the case of a read-only file). This could result in trying to seek before the beginning of the file, producing an error.
Here is a program that was affected:
#include <stdio.h>
int main(void) {
FILE *f = fopen("somefile","r");
if (!f) return 0;
fgetc(f);
ungetc('X', f);
fseek(f, 0, SEEK_CUR);
if (ferror(f)) puts("error encountered");
}
This maintains the invariant that these flags stay set to reflect the setting of the stream as read-only or write-only, allowing code elsewhere to behave appropriately based on that.
This should be allowed (for read/write files), but it was leading to errors, both because fputc would error out if the EOF flag was set and because the FILE record was not fully reset from its "reading" state. In particular, the _IOREAD flag and the current buffer position pointer need to be reset.
Here is a program that demonstrates the problem:
#include <stdio.h>
int main(void) {
FILE *f = fopen("somefile","r+");
if (!f) return 0;
while (!feof(f)) fgetc(f); /* or: fgetc then fread */
if (fputc('X', f) == EOF)
puts("fputc error");
}
This is needed to keep the file mark consistent with the state of the buffer. Otherwise, subsequent reads may return data from the wrong place. (Using fflush on a read-only stream is undefined behavior, but other things in stdio call it that way, so it must work consistently with what they expect.)
Here is an example that was affected by this:
#include <stdio.h>
#include <string.h>
char buf[100];
int main(void) {
FILE *f = fopen("somefile","r");
if (!f) return 0;
setvbuf(f, 0L, _IOFBF, 14);
fgets(buf, 10, f);
printf("first read : %s\n", buf);
ftell(f);
memset(buf, 0, sizeof(buf));
fgets(buf, 10, f);
printf("second read: %s\n", buf);
fseek(f, 0, SEEK_CUR);
memset(buf, 0, sizeof(buf));
fgets(buf, 10, f);
printf("third read : %s\n", buf);
}
This is not really an IO operation on the file, and accordingly the C standards do not describe the error indicator as being set here. In particular, you cannot get the mark for a character device, but that is expected behavior, not really an error condition.
errno is still set, indicating that the ftell operation failed.
ftell would set the mark as if the buffer and putback were being discarded, but not actually discard them. This resulted in characters being reread once the end of the buffer was reached.
Here is an example that illustrates the problem:
#include <stdio.h>
#include <string.h>
char buf[100];
int main(void) {
FILE *f = fopen("somefile","r"); // a file with some text
if (!f) return 0;
setvbuf(f, 0L, _IOFBF, 14); // to see problem sooner
fgets(buf, 10, f);
printf("first read : %s\n", buf);
ftell(f);
memset(buf, 0, sizeof(buf));
fgets(buf, 10, f);
printf("second read: %s\n", buf);]
}
If you read from a file using fgetc (or another function that calls it internally), and then later read from it using fread, any data left in the buffer from fgetc would be skipped over. The pattern causing this to happen was as follows:
fread called ~SetFilePointer, which (if there was buffered data) called fseek(f, 0, SEEK_CUR). fseek would call fflush, which calls ~InitBuffer. That zeros out the buffer count. fseek then calls ftell, which gets the current mark from GS/OS and then subtracts the buffer count. But the count was set to 0 in ~InitBuffer, so ftell reflects the position after any previously buffered data. fseek sets the mark to the position returned by ftell, i.e. after any data that was previously read and buffered, so that data would get skipped over. (Before commits 0047b755c9 and c95bfc19fb the behavior would be somewhat different due to the issue with ~InitBuffer that they addressed, but you could still get similar symptoms.)
The fix is simply to use the buffered data (if any), rather than discarding it.
Here is a test program illustrating the problem:
#include <stdio.h>
char buf[BUFSIZ+1];
#define RECSIZE 2
int main(void) {
FILE *f = fopen("somefile","r"); // a file with some data
if (!f) return 0;
fgetc(f);
size_t count = fread(buf, RECSIZE, BUFSIZ/RECSIZE, f);
printf("read %zu records: %s\n", count, buf);
}
There was a problem with the fix in commit 0047b755c9660: an instruction should have had an immediate operand, but did not because it was missing the #. This might cause the code to behave incorrectly, depending on memory contents.
This includes changes to fread to fix two problems. The first is that fread would ignore and discard characters put back with ungetc(). The second is that it would generally return the wrong value if reading from stdin with an element size other than 1 (it would return the count of bytes, not elements).
This fixes#9 (the first problem mentioned above).
These are the stdio.asm changes that were present in the beta source code on Opus ][, but had been reverted in commit e3c0c962d4. As it turns out, the changes to stdio.asm were OK--the issue was simply that the definitions of stdin/stdout/stderr and the associated initialization code in vars.asm had not been updated to account for the new version of the FILE structure. That has now been done, allowing the changes to work properly.
This fixes#7.
This adds a check for the filename argument being null, in which case it bails out and returns NULL. Previously, it would just dereference the null pointer and treat the contents of memory location 0 as a filename, with unpredictable results. A null filename indicates freopen should try to reopen the same file with a different mode, but the degree of support for that is implementation-defined. We still don't really support it, but at least this will report the failure and avoid unpredictable behavior.
It also removes some unused code at the end, but still forces fputc and fgetc to be linked in. These are needed because there are weak references to them in putchar and getchar, which may need to be used if stdin/stdout have been reopened.
This is what the standards require. Note that the mark is only set to EOF when actually writing to the underlying file, not merely buffering data to write later. This is consistent with the usual POSIX implementation using O_APPEND.
This would seek to the wrong location, and in some cases try to seek before the beginning of the file, leading to an error condition.
The problem stemmed from fseek calling fflush, which sets up the stream's buffer state in a way appropriate for writing but not for reading. It then calls ftell, which (for a read-only stream) would misinterpret this state and miscalculate the current position.
The fix is to make fflush work correctly on a read-only stream, setting up the state for reading. (User code calling fflush on a read-only stream would be undefined behavior, but since fseek does it we need to make it work.)
This fixes#6.
Most files already used spaces, but three used tabs for indentation. These have been converted to use spaces. This allows the files to be displayed with proper formatting in modern editors and on GitHub. It also removes any dependency on SysTabs settings when assembling them.
The spacing in fpextra.asm was also modified to use standard column positions.
There are no non-whitespace changes in this commit.
The C standards require it to do this, in addition to calling fseek.
Here is a test that can show the issue (in a realistic program, the indicator would be set due to an actual IO error, but for testing purposes this just sets it explicitly):
#include <stdio.h>
int main(void) {
FILE *f = tmpfile();
if (!f) return 0;
f->_flag |= _IOERR;
if (!ferror(f)) puts("bad ferror");
rewind(f);
if (ferror(f)) puts("rewind does not reset ferror");
fclose(f);
}
The core loop uses the 65816's decimal mode to create a BCD representation of the number, then we convert that to ASCII to print it. The idea is based on some code from Kent Dickey, with adjustments to fit this codebase. This technique should be faster, and also should typically result in smaller code, since the long long division routine will not need to be linked into every program that uses printf.
The new m16.int64 file contains a new "negate8" macro (for 64-bit negation), as well as an improved version of "ph8". These have been added to the individual *.macros files, but if those are regenerated, m16.int64 should be used as an input to macgen ahead of m16.ORCA (which contains the original version of ph8).
EOF should not be returned in this case.
I think it shouldn't actually write anything to the destination in this case, but it currently does. That problem remains unfixed for the moment; addressing it would require us to have our own internal buffer.
Conversions using it will not actually work right yet (unless assignment is suppressed), because the necessary changes have not been made in SysFloat, but the infrastructure is now in place to allow for those changes.
This was not being reported unless assignment was suppressed, because the "sta ~eofFound" instruction was too late.
(Found based on test cases by Rich Felker.)
Code in ~scanf would call ~RemoveWord to remove the extra parameters, but ~RemoveWord is designed to be called from one of the ~Scan_... subroutines, which is (in effect) called by JSR and thus has an extra word on the stack for its return address. This stack misalignment caused ~RemoveWord to overwrite a word of the caller's stack when called from the code to remove extra parameters.
This could cause a crash in the following program:
#include <stdio.h>
void f(void) {
int a,b;
sscanf("Z", "%i%i", &a, &b);
}
int main(void) {
f();
}
This may cause scanf to return EOF, if no conversions have been done.
This seems to be the intent of the C standards, although the wording could be clearer. It is also consistent with all the other implementations I tested.
Stephen Heumann