Hey Ken,
I found some bugs in pbm2ap.c which caused it to core dump. (Using
file descriptors after fclose(), using a pointer uninitialized).
```diff
--- pbm2ap.c.orig 2024-08-07 01:14:02.873843472 -0700
+++ pbm2ap.c 2024-08-07 02:25:16.119954682 -0700
@@ -953,10 +953,10 @@
/*
===================================================================================
Open and read the PBM file, parse the header and determine the image
geometry.
-Also return the pointer to the first byte of data.
+Returns true if successful.
===================================================================================
*/
-FILE* open_pbm_file(char *filename, long *x, long* y, char** dataptr,
char** endptr)
+int open_pbm_file(char *filename, long *x, long* y, char** dataptr,
char** endptr)
{
FILE* in;
char * data, *ptr;
@@ -965,7 +965,7 @@
if ((in = fopen(filename, "rb")) == NULL)
{
printf("unable to open %s\n", filename);
- return NULL;
+ return 0;
}
/* Read the input data */
@@ -980,8 +980,7 @@
if (data[0] != 'P' || data[1] != '4')
{
printf("Invalid input format\n");
- fclose(in);
- return NULL;
+ return 0;
}
/* Test for file comment */
@@ -1011,7 +1010,7 @@
*dataptr = ptr;
*endptr = ptr + *y*(*x+7)/8;
- return in;
+ return 1;
}
/*
@@ -1021,9 +1020,7 @@
*/
int main(int argc, char* argv[])
{
- FILE* in;
FILE* out;
- FILE* maskfd;
long x, y, outx, outy;
char * maskfile = NULL;
char *data_ptr, *data_endptr;
@@ -1032,7 +1029,7 @@
int opt;
int size[5];
- char *mask_ptr, *mask_endptr;
+ char *mask_ptr=NULL, *mask_endptr=NULL;
long mask_x, mask_y;
while ((opt = getopt(argc, argv, "m:orsuv")) != -1)
@@ -1075,31 +1072,31 @@
}
- if (optind+1 >= argc)
- {
- fprintf(stderr, "Usage: %s [-rsuv] infile outfile\n", argv[0]);
- fprintf(stderr, "\n");
- fprintf(stderr, " -r Encode for OptROM. Uses ALL
bytes 0-FFh\n");
- fprintf(stderr, " Not suitable for use with
BASIC\n\n");
- fprintf(stderr, " -s Perform silent conversion\n\n");
- fprintf(stderr, " -u Encode using upper ASCII bytes
80h-FFh also\n\n");
- fprintf(stderr, " -v Be verbose during the
conversion\n\n");
- exit(EXIT_FAILURE);
- }
+ if (optind+1 >= argc)
+ {
+ fprintf(stderr, "Usage: %s [-rsuv] [-m maskfile] infile
outfile\n", argv[0]);
+ fprintf(stderr, "\n");
+ fprintf(stderr, " -m Mask file\n");
+ fprintf(stderr, " -r Encode for OptROM. Uses ALL
bytes 0-FFh\n");
+ fprintf(stderr, " Not suitable for use with
BASIC\n\n");
+ fprintf(stderr, " -s Perform silent conversion\n\n");
+ fprintf(stderr, " -u Encode using upper ASCII bytes
80h-FFh also\n\n");
+ fprintf(stderr, " -v Be verbose during the
conversion\n\n");
+ exit(EXIT_FAILURE);
+ }
// Open the input file
- if ((in = open_pbm_file(argv[optind], &x, &y, &data_ptr,
&data_endptr)) == NULL)
+ if (!open_pbm_file(argv[optind], &x, &y, &data_ptr, &data_endptr))
{
+ fprintf(stderr, "Unable to open file '%s'\n", argv[optind]);
return 1;
}
// If a mask file given, try to open it
if (maskfile)
{
- maskfd = open_pbm_file(maskfile, &mask_x, &mask_y, &mask_ptr,
&mask_endptr);
-
// Validate we can open the maskfile
- if (maskfd == NULL)
+ if (!open_pbm_file(maskfile, &mask_x, &mask_y, &mask_ptr,
&mask_endptr))
{
fprintf(stderr, "Unable to open mask file %s\n", maskfile);
return 1;
@@ -1130,7 +1127,6 @@
if ((out = fopen(argv[optind + 1], "wb+")) == NULL)
{
printf("unable to open %s\n", argv[optind + 1]);
- fclose(in);
return 1;
}
```
—b9
On Tue, Aug 6, 2024 at 8:08 AM Kenneth Pettit <[email protected]> wrote:
On 8/6/24 7:46 AM, Joshua O'Keefe wrote:
On Aug 6, 2024, at 5:33 AM, Kenneth Pettit <[email protected]>
<mailto:[email protected]> wrote:
mask = ~(0x01 << data);
Wow, Ken, it's amazing how you can just zoom right into a spot in
the code and find this so quickly based on the bug description.
That's the sign of a man who knows his codebase well...
Thanks Joshua, though it was B 9's image that told me *exactly*
what the problem was and where to look. :) Seeing the triangle
patterns, it was clear the code was clearing too many bits
(pixels) from the emulated LCD RAM byte.
Ken
