Getting a runtime error::"The instruction at 0x004010b4 referencedmemory at 0x00000000. The memory c

M

Maxx

Ok here is this program to compress a given file using run-length
encoding. This technique reduces the size of the file by replacing the
original bytes with the repetition count and the byte to be repeated.
Ex: if the file to be compressed starts with the following sequence of
bytes::
46 6F 6F 6F 2B 2B 9C 81 81 81 81 84 84 84 7A 7A 7A

The compressed file will contain the following bytes::
01 46 03 6F 02 2B 01 9C 04 81 03 84 03 7A

And the output file will have a .rle extension along with the original
file name that was provided when the program was run. here is the
program i wrote:::

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>

#define MAXBUFF 5000

int main(int argc, char *argv[])
{
FILE *fp,*gp;
int ch, a[MAXBUFF],o[MAXBUFF],n=0,i,k;
int *p=a,*r=a,*q=o;
if(argc ==1)
{
fprintf(stderr,"Too few arguments");
exit(1);
}
if(fp=fopen(*++argv,"rb")==NULL)
{
fprintf(stderr,"Can't open file",*argv);
exit(2);
}

if(gp=fopen(*argv[1]+".rle","wb")==NULL)
{
fprintf(stderr,"Can't open file");
exit(2);
}
while(fscanf(fp,"%.2X",&ch)==1)
{
*p++=ch;
++n;
}*p='\0';

for(i=0;i<n;i++)
{
int j=*(r+i),m=0;
for(k=0;k<n;k++)
{
if(j==*(r+k))
{
++m;
}
}
*q++=m;
*q++=j;
}*q='\0';
while(*q!='\0')
{
putc(*q,gp);
q++;
}
}

It got compiled well but when i run it it gives me the following
error::"The instruction at "0x004010b4" referenced memory at
"0x00000000". The memory could not be read."
Please help, i can't seem to figure it out
 
J

Jens Thoms Toerring

Maxx said:
Ok here is this program to compress a given file using run-length
encoding. This technique reduces the size of the file by replacing the
original bytes with the repetition count and the byte to be repeated.
Ex: if the file to be compressed starts with the following sequence of
bytes::
46 6F 6F 6F 2B 2B 9C 81 81 81 81 84 84 84 7A 7A 7A
The compressed file will contain the following bytes::
01 46 03 6F 02 2B 01 9C 04 81 03 84 03 7A

Do you realize that in the worst possible case (when no bytes
are ever repeated) this will result in a file twice as long as
the original one? It looks as if you don't since your output
buffer isn't larger than that for the input...

There is also another case that you don't seem to consider: what
do you do if there are more than 255 repetitions?
And the output file will have a .rle extension along with the original
file name that was provided when the program was run. here is the
program i wrote:::
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#define MAXBUFF 5000
int main(int argc, char *argv[])
{
FILE *fp,*gp;
int ch, a[MAXBUFF],o[MAXBUFF],n=0,i,k;
int *p=a,*r=a,*q=o;
if(argc ==1)
{
fprintf(stderr,"Too few arguments");
exit(1);
}
if(fp=fopen(*++argv,"rb")==NULL)
{
fprintf(stderr,"Can't open file",*argv);
exit(2);
}
if(gp=fopen(*argv[1]+".rle","wb")==NULL)
{
fprintf(stderr,"Can't open file");
exit(2);
}

The problems with the above have already been pointed out,
i.e. that you don't get any file pointers into 'fp' and
'gp' (the compiler should have warned you about that if
you had used suitable warning flags...) That probably is
already is the reason for the error message you got. But
things don't get any better afterwards.
while(fscanf(fp,"%.2X",&ch)==1)

I guess that should be

while ( fscanf( fp, "%2X", &ch ) == 1 )

but then 'ch' should be an unsigned int - that's what you tell
fscanf() via "%X". Of course, 'p' should then also be pointer
to unsigned int and 'a' an array of unsigned ints.

But the main problem would appear to be that you seem to be
reading a binary file (or why open it explicitely with "rb"?)
but you are reading it as if it would contain ASCII data,
which is what fscanf() is meant for. If you want to read data
from a binary file use fread().
{
*p++=ch;
++n;
}*p='\0';

How do you protect against reading more than MAXBUFF-1 times?
You don't so there's no protection against writing past the
end of the 'a' buffer. And what's it good for to append a 0 to
the end? That looks like cargo-cult programming...

Since you're done now with the input file you should close
it now.
for(i=0;i<n;i++)
{
int j=*(r+i),m=0;
for(k=0;k<n;k++)
{
if(j==*(r+k))
{
++m;
}
}

This has obvioulsy nothing to do with your stated goal of
doing run-length encoding. You count here how often the
value at position 'i' appears in _all_ of the buffer (and
that for _all_ positions in the input buffer). To make any
sense at all you would need something similar to

for ( i = 0; i < n; i = k )
{
int j = a[ i ],
m = 1;
for ( k = i + 1; k < n && j == a[ k ]; k++ )
m++;
i = k;

This stil doesn't address the problem of a byte being
repeated more than 255 times, that would maje things a
bit more interesting;-)

Note that I use the original array 'a' instead of the extra
pointer 'r' - using pointers just for the sake of using poin-
ters won't magically make your program any faster or better
in other respects, just harder to read.
*q++=m;
*q++=j;

The way you have written the program the output buffer always
needs to be twice as large as the input buffer...
}*q='\0';
while(*q!='\0')
{
putc(*q,gp);
q++;
}
}

Well, to begin with you start the loop with 'q' pointing
to the last element written to (with 0), so this loop
will never be run (you would need to reset 'q' first to
point again to the start of the 'o' buffer).

But also then this again would hardly any sense since you
don't seem to grasp the difference between binary and ASCII
data (putc() is for ASCII data, but you are dealing with bi-
nary) it also fails because one of the data values in the
output array could be 0, thus making you stop prematurely -
adding a 0 at the end makes only sense when you're dealing
with strings (which can't contain any '\0'), not for other
data.

And when you're done you should also close the output
file and return an int from the program (either 0 or
EXIT_SUCCESS would come to mind).

Regards, Jens
 
J

James Dow Allen

        if(fp=fopen(*++argv,"rb")==NULL) /* bug */
[more code whose functionality, or lack thereof,
is irrelevant]

My comment is intended not just for Maxx, but for
Arnuld and a few others who post here.

I've been programming for quite a while now, and make
relatively few mistakes, yet I *STILL* will compile
and run code before it's finished, just to do piecemeal
verification. Perhaps I should be embarrassed to
admit it, but sometimes I even have something like
printf("Last arg is *%s*\n", argv[argc-1]);
just to make sure I've not gone off-by-one somewhere.

Yet we see complicated programs posted with the
first line wrong! :)

Long ago I worked in a 24-hour turnaround environment:
code as much as you can accurately in a day, and
study the coredump because *you only get one
compile-and-go per day!* Those days are gone now.
Use the compiler interactively and let it
be your friend!

James Dow Allen
 
M

Maxx

 Maxx said:
error::"The instruction at "0x004010b4" referenced memory at
"0x00000000". The memory could not be read."

You dereferenced a NULL.
   if(fp=fopen(*++argv,"rb")==NULL)

This is equivalent to
        if ( fp = (fopen(*++argv,"rb")==NULL) )

You probably meant
   if((fp=fopen(*++argv,"rb"))==NULL)
   if(gp=fopen(*argv[1]+".rle","wb")==NULL)

This adds two addresses instead of appending a string. To append a string you
have to allocate the string and then write into it.

    char name[strlen(argv[1])+5];
    sprintf(name, "%s.rle", argv[1]);

and then as above

    if((gp=fopen(name,"wb"))==NULL)

--
Damn the living - It's a lovely life.           I'm whoever you want me to be.
Silver silverware - Where is the love?       At least I can stay in character.
Oval swimming pool - Where is the love?    Annoying Usenet one post at a time.
Damn the living - It's a lovely life.      And ŒAll is vane¹ the **** replied.


Oh i get it, i forgot to put the braces. And that string concatenation
was exactly the thing i was looking for.Thanks a lot
 
M

Maxx

Maxx said:
Ok here is this program to compress a given file using run-length
encoding. This technique reduces the size of the file by replacing the
original bytes with the repetition count and the byte to be repeated.
Ex: if the file to be compressed starts with the following sequence of
bytes::
46 6F 6F 6F 2B 2B 9C 81 81 81 81 84 84 84 7A 7A 7A
The compressed file will contain the following bytes::
01 46 03 6F 02 2B 01 9C 04 81 03 84 03 7A

Do you realize that in the worst possible case (when no bytes
are ever repeated) this will result in a file twice as long as
the original one? It looks as if you don't since your output
buffer isn't larger than that for the input...

There is also another case that you don't seem to consider: what
do you do if there are more than 255 repetitions?


And the output file will have a .rle extension along with the original
file name that was provided when the program was run. here is the
program i wrote:::
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#define MAXBUFF 5000
int main(int argc, char *argv[])
{
        FILE *fp,*gp;
        int ch, a[MAXBUFF],o[MAXBUFF],n=0,i,k;
        int *p=a,*r=a,*q=o;
        if(argc ==1)
        {
                fprintf(stderr,"Too few arguments");
                exit(1);
        }
        if(fp=fopen(*++argv,"rb")==NULL)
        {
                fprintf(stderr,"Can't open file",*argv);
                exit(2);
        }
        if(gp=fopen(*argv[1]+".rle","wb")==NULL)
        {
                fprintf(stderr,"Can't open file");
                exit(2);
        }

The problems with the above have already been pointed out,
i.e. that you don't get any file pointers into 'fp' and
'gp' (the compiler should have warned you about that if
you had used suitable warning flags...) That probably is
already is the reason for the error message you got. But
things don't get any better afterwards.
        while(fscanf(fp,"%.2X",&ch)==1)

I guess that should be

         while ( fscanf( fp, "%2X", &ch ) == 1 )

but then 'ch' should be an unsigned int - that's what you tell
fscanf() via "%X". Of course, 'p' should then also be pointer
to unsigned int and 'a' an array of unsigned ints.

But the main problem would appear to be that you seem to be
reading a binary file (or why open it explicitely with "rb"?)
but you are reading it as if it would contain ASCII data,
which is what fscanf() is meant for. If you want to read data
from a binary file use fread().
        {
                *p++=ch;
                ++n;
        }*p='\0';

How do you protect against reading more than MAXBUFF-1 times?
You don't so there's no protection against writing past the
end of the 'a' buffer. And what's it good for to append a 0 to
the end? That looks like cargo-cult programming...

Since you're done now with the input file you should close
it now.
        for(i=0;i<n;i++)
        {
                int j=*(r+i),m=0;
                for(k=0;k<n;k++)
                {
                        if(j==*(r+k))
                        {
                                ++m;
                        }
                }

This has obvioulsy nothing to do with your stated goal of
doing run-length encoding. You count here how often the
value at position 'i' appears in _all_ of the buffer (and
that for _all_ positions in the input buffer). To make any
sense at all you would need something similar to

            for ( i = 0; i < n; i = k )
            {
                 int j = a[ i ],
                                     m = 1;
                 for ( k = i + 1; k < n && j == a[ k ]; k++ )
                      m++;
                 i = k;

This stil doesn't address the problem of a byte being
repeated more than 255 times, that would maje things a
bit more interesting;-)

Note that I use the original array 'a' instead of the extra
pointer 'r' - using pointers just for the sake of using poin-
ters won't magically make your program any faster or better
in other respects, just harder to read.
                *q++=m;
                *q++=j;

The way you have written the program the output buffer always
needs to be twice as large as the input buffer...
        }*q='\0';
        while(*q!='\0')
        {
                putc(*q,gp);
                q++;
        }
}

Well, to begin with you start the loop with 'q' pointing
to the last element written to (with 0), so this loop
will never be run (you would need to reset 'q' first to
point again to the start of the 'o' buffer).

But also then this again would hardly any sense since you
don't seem to grasp the difference between binary and ASCII
data (putc() is for ASCII data, but you are dealing with bi-
nary) it also fails because one of the data values in the
output array could be 0, thus making you stop prematurely -
adding a 0 at the end makes only sense when you're dealing
with strings (which can't contain any '\0'), not for other
data.

And when you're done you should also close the output
file and return an int from the program (either 0 or
EXIT_SUCCESS would come to mind).

                            Regards, Jens



Yeah i was aware of the worst possible scenario. actually i found this
program in that K. N. King c programming book, the best case and the
worst case was pointed out by the author.
and that 255 repetition thing could have sent my program haywire, i
realized it afterwards.

Actually i by binary i thought it included all types of files, more
like a super-set.So i was aiming to read a pair of bytes("2X") from
the input file and store them in a array.
i should have used array indices there, my mistake, that would really
be a lot easier. And thanks a lot for correcting the program.
 
M

Maxx

        if(fp=fopen(*++argv,"rb")==NULL) /* bug */
 [more code whose functionality, or lack thereof,
  is irrelevant]

My comment is intended not just for Maxx, but for
Arnuld and a few others who post here.

I've been programming for quite a while now, and make
relatively few mistakes, yet I *STILL* will compile
and run code before it's finished, just to do piecemeal
verification.  Perhaps I should be embarrassed to
admit it, but sometimes I even have something like
    printf("Last arg is *%s*\n", argv[argc-1]);
just to make sure I've not gone off-by-one somewhere.

Yet we see complicated programs posted with the
first line wrong!  :)

Long ago I worked in a 24-hour turnaround environment:
code as much as you can accurately in a day, and
study the coredump because *you only get one
compile-and-go per day!*  Those days are gone now.
Use the compiler interactively and let it
be your friend!

James Dow Allen

I'm very new in programming. only have been learning C past 2 months
now. And these sort of mistakes keep coming up, although i try to
correct most of the compiler errors but run time error drive me crazy,
i just couldn't figure out how to solve it. Thanks to usenet and
thanks to this group i started learning more about these errors and
more about C
 
J

Jens Thoms Toerring

Maxx said:
Yeah i was aware of the worst possible scenario. actually i found this
program in that K. N. King c programming book, the best case and the
worst case was pointed out by the author.

Well, when you're programming you always have to cater for the
worst possible case, so if you know that the worst case is that
the output buffer could potentially be twice as large as the
input buffer then you need to make it twice as large (or come
up with some clever trick to avoid the problem some other way)...
and that 255 repetition thing could have sent my program haywire, i
realized it afterwards.

The most simple way to get around that is probably to stop
counting when you have found 255 repetitions and simply write
that out and continue, pretending that you found a "new" value
after the 255 repetitions. Of course, the program that re-
translates from the run-lenth encoded file back into a normal
file must expect this behaviour.
Actually i by binary i thought it included all types of files, more
like a super-set.So i was aiming to read a pair of bytes("2X") from
the input file and store them in a array.

What you actually seem to want to read is a single byte. When
you write in your post e.g.
46 6F 6F 6F 2B 2B 9C 81 81 81 81 84 84 84 7A 7A 7A

then the "6F" is a human readable representation of a single
byte with the bit pattern '01101111' (the first four bits can
be seen as a 6 and the second set of four bits as 15 or hexa-
decimal 'F'). But a file doesn't contain human readable repre-
sentations but real "raw" bytes. And when you do run-length
encoding you count how many bytes with the same value are in
the file and replace that with a count and the bytes value.
So you are supposed to read single bytes and count them. And if
you read "raw" bytes it's nearly always more appropriate to use
functions like fread(). A function like fscanf() when used with
"%2X" expects to find a hexadecimal ASCII representation (some-
thing literally like the two characters '6' and 'F') in the file
and converts that to a "raw" byte for use with the program. But
the file already contains "raw" bytes you want to read, so

unsigned char c;
fread( &c, 1, 1, fp );

is the way to get that into 'c' (an unsigned char is a good
type of variable to hold "raw" bytes) with a value that may
have a human readable hexadecimal representation like "6F".

And then you would have another unsigned char, let's say named
'count', for counting how many bytes of the same value you read
(the problem with 255 stems from an unsigned char not being
guaranteed to be able to hold a larger value). So you read
single bytes until you arrive at one that's different from
the ones before (or you reach the end of the file!) and then
you write both the count and the value by e.g.

fwrite( &count, 1, 1, gp );
fwrite( &c, 1, 1, gp );

That will write two "raw" bytes in the file, first the count
and then the byte's value itself.

I would recommend that you start this simple and, for the first
version of the program, just read single bytes and deal with
them. When that works (you should also write a program that
reads in the encoded file and converts it back to the original
one, so you can check if things work correctly) as the next
step you can consider how to read in larger chunks of bytes in
one go. This will make the program probably quite a bit faster,
but it's also more complicated - at least you got a lot more
chances to make mistakes;-) But before you start with that it's
important that you get a firm grasp of how values are stored in
a file and the difference between e.g. the ASCII representation
of a value and what's really in the file.

Regards, Jens
 
M

Maxx

Well, when you're programming you always have to cater for the
worst possible case, so if you know that the worst case is that
the output buffer could potentially be twice as large as the
input buffer then you need to make it twice as large (or come
up with some clever trick to avoid the problem some other way)...


The most simple way to get around that is probably to stop
counting when you have found 255 repetitions and simply write
that out and continue, pretending that you found a "new" value
after the 255 repetitions. Of course, the program that re-
translates from the run-lenth encoded file back into a normal
file must expect this behaviour.


What you actually seem to want to read is a single byte. When
you write in your post e.g.


then the "6F" is a human readable representation of a single
byte with the bit pattern '01101111' (the first four bits can
be seen as a 6 and the second set of four bits as 15 or hexa-
decimal 'F'). But a file doesn't contain human readable repre-
sentations but real "raw" bytes. And when you do run-length
encoding you count how many bytes with the same value are in
the file and replace that with a count and the bytes value.
So you are supposed to read single bytes and count them. And if
you read "raw" bytes it's nearly always more appropriate to use
functions like fread(). A function like fscanf() when used with
"%2X" expects to find a hexadecimal ASCII representation (some-
thing literally like the two characters '6' and 'F') in the file
and converts that to a "raw" byte for use with the program. But
the file already contains "raw" bytes you want to read, so

       unsigned char c;
       fread( &c, 1, 1, fp );

is the way to get that into 'c' (an unsigned char is a good
type of variable to hold "raw" bytes) with a value that may
have a human readable hexadecimal representation like "6F".

And then you would have another unsigned char, let's say named
'count', for counting how many bytes of the same value you read
(the problem with 255 stems from an unsigned char not being
guaranteed to be able to hold a larger value). So you read
single bytes until you arrive at one that's different from
the ones before (or you reach the end of the file!) and then
you write both the count and the value by e.g.

       fwrite( &count, 1, 1, gp );
       fwrite( &c, 1, 1, gp );

That will write two "raw" bytes in the file, first the count
and then the byte's value itself.

I would recommend that you start this simple and, for the first
version of the program, just read single bytes and deal with
them. When that works (you should also write a program that
reads in the encoded file and converts it back to the original
one, so you can check if things work correctly) as the next
step you can consider how to read in larger chunks of bytes in
one go. This will make the program probably quite a bit faster,
but it's also more complicated - at least you got a lot more
chances to make mistakes;-) But before you start with that it's
important that you get a firm grasp of how values are stored in
a file and the difference between e.g. the ASCII representation
of a value and what's really in the file.

                              Regards, Jens

Ok
 
M

Maxx

Well, when you're programming you always have to cater for the
worst possible case, so if you know that the worst case is that
the output buffer could potentially be twice as large as the
input buffer then you need to make it twice as large (or come
up with some clever trick to avoid the problem some other way)...


The most simple way to get around that is probably to stop
counting when you have found 255 repetitions and simply write
that out and continue, pretending that you found a "new" value
after the 255 repetitions. Of course, the program that re-
translates from the run-lenth encoded file back into a normal
file must expect this behaviour.


What you actually seem to want to read is a single byte. When
you write in your post e.g.


then the "6F" is a human readable representation of a single
byte with the bit pattern '01101111' (the first four bits can
be seen as a 6 and the second set of four bits as 15 or hexa-
decimal 'F'). But a file doesn't contain human readable repre-
sentations but real "raw" bytes. And when you do run-length
encoding you count how many bytes with the same value are in
the file and replace that with a count and the bytes value.
So you are supposed to read single bytes and count them. And if
you read "raw" bytes it's nearly always more appropriate to use
functions like fread(). A function like fscanf() when used with
"%2X" expects to find a hexadecimal ASCII representation (some-
thing literally like the two characters '6' and 'F') in the file
and converts that to a "raw" byte for use with the program. But
the file already contains "raw" bytes you want to read, so

       unsigned char c;
       fread( &c, 1, 1, fp );

is the way to get that into 'c' (an unsigned char is a good
type of variable to hold "raw" bytes) with a value that may
have a human readable hexadecimal representation like "6F".

And then you would have another unsigned char, let's say named
'count', for counting how many bytes of the same value you read
(the problem with 255 stems from an unsigned char not being
guaranteed to be able to hold a larger value). So you read
single bytes until you arrive at one that's different from
the ones before (or you reach the end of the file!) and then
you write both the count and the value by e.g.

       fwrite( &count, 1, 1, gp );
       fwrite( &c, 1, 1, gp );

That will write two "raw" bytes in the file, first the count
and then the byte's value itself.

I would recommend that you start this simple and, for the first
version of the program, just read single bytes and deal with
them. When that works (you should also write a program that
reads in the encoded file and converts it back to the original
one, so you can check if things work correctly) as the next
step you can consider how to read in larger chunks of bytes in
one go. This will make the program probably quite a bit faster,
but it's also more complicated - at least you got a lot more
chances to make mistakes;-) But before you start with that it's
important that you get a firm grasp of how values are stored in
a file and the difference between e.g. the ASCII representation
of a value and what's really in the file.

                              Regards, Jens

Ok so i need to use fwrit() to write raw bytes, got it. Actually i
thought fread()/fwrite() were used to read large blocks of file.
Anyways thanks for your help.
 
J

Jens Thoms Toerring

Maxx said:
Ok so i need to use fwrit() to write raw bytes, got it. Actually i
thought fread()/fwrite() were used to read large blocks of file.

You can use them to read/write as many bytes as you need.
They work for single bytes as well as for gigabytes. Of
course, each call of such a function takes a bit of time,
so it may be more efficient to read in larger chunks and
thus, in the long run, one probably might be looking for
a way to reduce the number of calls - but only after the
algorithm that's applied to the data has been tested and
proven to be working correctly. Try to get it right first
and only then attempt to optimize;-)

Actually, if you write your program to have two func-
tions, one for dealing with input and the other with
output, like for example

int get_a_char( unsigned char * c );

and

void output_rle_data( unsigned char count,
unsigned char value );

you can use them within your program to get new fresh bytes
on which you apply your encoding algorithm and then to send
out a new set of a count and a value, regardless of how
exactly this is done.

In a first version you might read just single bytes from
a file at a time with get_a_char() and directly output
encoded data to another file with output_rle_data(). I.e.
get_a_char() at first could be as simple as

int get_a_char( unsigned char *c )
{
return fread( c, 1, 1, fp );
}

In the caller (i.e. the part that does the run-length
encoding) you check if the return value is 1 and then
you know you got a new byte to deal with, otherwise
you have reached the end of the file (or a read error
occured).

If you later want to get more fancy you can change just
this function so that it reads in data in larger chunks.
You could e.g make that function store them in a local
buffer and return single bytes from that buffer to the
caller, reading more from the file only when the buffer
becomes empty. But the "interface" for the caller stays
exactly the same as before, i.e. the caller continues to
receive a new byte via the pointer argument as long as
the return value isn't 0.

The advantage is that you then don't have to change
anything about your "core algorithm" that deals with
the run-length encoding and which you know works cor-
rectly. So any new bugs must be in the input function
which makes it lot easier to debug the program. As an
additional benefit it also will be easier for others
to understand the program (and also for yourself when
you read it again in half a years time).

The same, of course, holds for the output function.

Regards, Jens
 
M

Maxx

You can use them to read/write as many bytes as you need.
They work for single bytes as well as for gigabytes. Of
course, each call of such a function takes a bit of time,
so it may be more efficient to read in larger chunks and
thus, in the long run, one probably might be looking for
a way to reduce the number of calls - but only after the
algorithm that's applied to the data has been tested and
proven to be working correctly. Try to get it right first
and only then attempt to optimize;-)

Actually, if you write your program to have two func-
tions, one for dealing with input and the other with
output, like for example

  int get_a_char( unsigned char * c );

and

  void output_rle_data( unsigned char count,
                        unsigned char value );

you can use them within your program to get new fresh bytes
on which you apply your encoding algorithm and then to send
out a new set of a count and a value, regardless of how
exactly this is done.

In a first version you might read just single bytes from
a file at a time with get_a_char() and directly output
encoded data to another file with output_rle_data(). I.e.
get_a_char() at first could be as simple as

int get_a_char( unsigned char *c )
{
        return fread( c, 1, 1, fp );

}

In the caller (i.e. the part that does the run-length
encoding) you check if the return value is 1 and then
you know you got a new byte to deal with, otherwise
you have reached the end of the file (or a read error
occured).

If you later want to get more fancy you can change just
this function so that it reads in data in larger chunks.
You could e.g make that function store them in a local
buffer and return single bytes from that buffer to the
caller, reading more from the file only when the buffer
becomes empty. But the "interface" for the caller stays
exactly the same as before, i.e. the caller continues to
receive a new byte via the pointer argument as long as
the return value isn't 0.

The advantage is that you then don't have to change
anything about your "core algorithm" that deals with
the run-length encoding and which you know works cor-
rectly. So any new bugs must be in the input function
which makes it lot easier to debug the program. As an
additional benefit it also will be easier for others
to understand the program (and also for yourself when
you read it again in half a years time).

The same, of course, holds for the output function.

                       Regards, Jens


I have rewritten the program using the guidelines that you've
provided, and it compiled well it didn't gave any runtime errors, but
the funny thing that happened is that the size of the resultant have
grown exactly twice. here is the program::

#include <stdio.h>
#include <stdlib.h>

#define MAXBUFF 5000
FILE *fp,*gp;
int get_a_char(unsigned char *);
void output_rle_data(unsigned char, unsigned char);

int main(int argc, char *argv[])
{

int n=0,i,m,j,k;
char name[20];
unsigned char c,count,a[MAXBUFF],*p=a;
sprintf(name,"%s.rle",argv[1]);
if(argc ==1)
{
fprintf(stderr,"Too few arguments");
exit(1);
}
if((fp=fopen(*++argv,"rb")) ==NULL)
{
fprintf(stderr,"Can't open file",*argv);
exit(2);
}

if((gp=fopen(name,"wb"))==NULL)
{
fprintf(stderr,"Can't open file");
exit(2);
}
while((m=get_a_char(&c)) ==1)
{
*p++=c;
++n;
}*p='\0';

for(i=0;i<n;i++)
{
j=a,count=0;
for(k=0;k<n;k++)
{
if(j=a[k])
++count;
}
output_rle_data(count,j);
}
}

int get_a_char(unsigned char *c)
{
return fread(&c, 1, 1, fp);
}

void output_rle_data(unsigned char cn, unsigned char jn)
{
fwrite(&cn,1,1,gp);
fwrite(&jn,1,1,gp);
}
 
B

Barry Schwarz

On Mon, 27 Dec 2010 07:41:39 -0800 (PST), Maxx

snip
I have rewritten the program using the guidelines that you've
provided, and it compiled well it didn't gave any runtime errors, but

It still produces the wrong result. Not all errors are caught by the
system.
the funny thing that happened is that the size of the resultant have
grown exactly twice. here is the program::

Since you write two bytes to your output file for every byte of your
input file, this is the only acceptable result. What were you
expecting?
#include <stdio.h>
#include <stdlib.h>

#define MAXBUFF 5000
FILE *fp,*gp;
int get_a_char(unsigned char *);
void output_rle_data(unsigned char, unsigned char);

int main(int argc, char *argv[])
{

int n=0,i,m,j,k;
char name[20];
unsigned char c,count,a[MAXBUFF],*p=a;
sprintf(name,"%s.rle",argv[1]);

How do you insure that the user never provides a command line
parameter longer than 15 characters? For that matter, how do you
insure that argv[1] exists and is not NULL?
if(argc ==1)
{

If you reach this point, the previous call to sprintf has already
invoked undefined behavior.
fprintf(stderr,"Too few arguments");
exit(1);

1 is not a portable return value from main. Use EXIT_FAILURE which
is.
}
if((fp=fopen(*++argv,"rb")) ==NULL)
{
fprintf(stderr,"Can't open file",*argv);

What purpose do you think the third argument serves. fprintf will
never look at it. Did you mean to include a %s in your format string?

2 is not portable either.
}

if((gp=fopen(name,"wb"))==NULL)
{
fprintf(stderr,"Can't open file");

Wouldn't it be nice if you told the user which file could not be
opened?
exit(2);
}
while((m=get_a_char(&c)) ==1)
{
*p++=c;

Due to an error in get_a_char, c is not assigned the character read
from the file. c is not initialized or assigned to so its value is
indeterminate. Your array a will be populated with garbage.

After you fix get_a_char, you still need to insure you do not overrun
the array a.
++n;
}*p='\0';

To avoid confusion, this assignment should be on a new line.
for(i=0;i<n;i++)
{
j=a,count=0;
for(k=0;k<n;k++)
{
if(j=a[k])
++count;


Indenting consistently has one of the best cost/benefit ratios of all
style advice. You would also be better off not using tabs in posted
code.

Why are you using an unsigned char to count occurrences. Your array
holds 5000 elements. Surely one value can occur more than 255 times.
}
output_rle_data(count,j);

You are aware that if a value appears multiple times in the array, you
will output its count each time.

main() returns an int and you should do so explicitly.
}

int get_a_char(unsigned char *c)
{
return fread(&c, 1, 1, fp);

While this cannot produce a diagnostic because the first parameter to
fread is a void* and therefore any pointer argument is valid, you have
provided the wrong value. You wanted fread to store the character
read from the file into the char whose address you passed in c. That
would be the unsigned char c defined in main. By using the same name
here, you managed to confuse yourself. The argument you actually pass
to fread is the address of the local pointer in this function. As a
result, fread will store the character read from the file in the first
byte of this pointer and not in the char defined in main. Delete the
&.
 
J

Jens Thoms Toerring

Maxx said:
I have rewritten the program using the guidelines that you've
provided, and it compiled well it didn't gave any runtime errors, but
the funny thing that happened is that the size of the resultant have
grown exactly twice. here is the program::
#include <stdio.h>
#include <stdlib.h>
#define MAXBUFF 5000
FILE *fp,*gp;
int get_a_char(unsigned char *);
void output_rle_data(unsigned char, unsigned char);
int main(int argc, char *argv[])
{
int n=0,i,m,j,k;
char name[20];

That may be much too short!
unsigned char c,count,a[MAXBUFF],*p=a;
sprintf(name,"%s.rle",argv[1]);

If the file name passed as the argument is has more than
15 characters thos will write past the end of the 'name'
array. Since you can't forsee how long the file name the
user inputs is going to be using a static buffer is rather
problematic - you would have to abbreiviate it if it's
too long (which might result in surprises for the user.
Better use a dynamically allocated buffer.
if(argc ==1)
{
fprintf(stderr,"Too few arguments");
exit(1);

Doing a return here will get you out also. And, while you're
free to return any integer value in principle, the only values
guaranteed to work everywere are EXIT_SUCESS and EXIT_FAILURE.
}
if((fp=fopen(*++argv,"rb")) ==NULL)

Why the strange "gymnastics" with the poor argv pointer?
{
fprintf(stderr,"Can't open file",*argv);
exit(2);
}
if((gp=fopen(name,"wb"))==NULL)
{
fprintf(stderr,"Can't open file");
exit(2);
}
while((m=get_a_char(&c)) ==1)
{
*p++=c;
++n;
}*p='\0';

This is again very problematic. Consider what will happen if
the input file is longer than MAXBUFF-1: you will write past
the end of the 'a' array! And putting a '\0' in the last after
the last element is absolutely useless - this isn't a string.

Also, what I proposed was not to use any buffers in a first
version of the program. And if you use buffers it's not very
clever to use them in a way that restricts the possible size
of the files you can deal with. But if you do that you then
must check for longer files and find some way to deal with
the situation...
for(i=0;i<n;i++)
{
j=a,count=0;
for(k=0;k<n;k++)
{
if(j=a[k])
++count;
}
output_rle_data(count,j);
}


Well, what you do here ins't run-length encoding at all. What
you do is counting for each position in the file how many bytes
exist in the whole file with the same value as at that position.
And then you write out the count and the value. Of course, the
resulting file will be twice as large as the input file. (And
the count may be wrong for a number of positions when the counter
overflowed).

Run-length encoding, as you described it yourself in your first
post, means something different: you count how many bytes of
the same value are in the file in the file, write that out and
then go on with the next set of bytes of the same value.

And then you're missing to return an int at the end;-)
int get_a_char(unsigned char *c)
{
return fread(&c, 1, 1, fp);

Since you pass a pointer to the function this must be

return fread( c, 1, 1, fp );

Note the removed '&' in front of 'c'.
void output_rle_data(unsigned char cn, unsigned char jn)
{
fwrite(&cn,1,1,gp);
fwrite(&jn,1,1,gp);
}

Here's a version ofhow I might do it:

--------------8<----------------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

static FILE *in_fp,
*out_fp;

static void rle_encode( void );
static int get_a_byte( unsigned char * );
static void output_rle_data( unsigned char,
unsigned char );

int main( int argc,
char * argv[ ] )
{
char *name;

if ( argc < 2 )
{
fprintf( stderr, "Too few arguments\n" );
return EXIT_FAILURE;
}

if ( ( in_fp = fopen( argv[ 1 ], "rb" ) ) == NULL )
{

fprintf( stderr, "Can't open input file '%s'\n", argv[ 1 ] );
return EXIT_FAILURE;
}

if ( ( name = malloc( strlen( argv[ 1 ] ) + 5 ) ) == NULL )
{
fprintf( stderr, "Running out of memory\n" );
fclose( in_fp );
return EXIT_FAILURE;
}

sprintf( name, "%s.rle", argv[ 1 ] );

if ( ( out_fp = fopen( name, "wb" ) ) == NULL )
{
fprintf( stderr,"Can't open output file '%s'\n", name );
free( name );
fclose( in_fp );
return EXIT_FAILURE;
}

free( name );

rle_encode( );

fclose( out_fp );
fclose( in_fp );
return EXIT_SUCCESS;
}

static void
rle_encode( void )
{
unsigned char count = 1,
curr,
next = 0;
int res;

/* Get the very first byte from the file */

res = get_a_byte( &curr );

/* Keep going while something could be read in */

while ( res != 0 )
{
/* Get a new byte from the file - stop if we already read 255 bytes
of the same value, if nothing could be read anymore or if the new
bytes value differs from the old one */

while ( count < 255 && ( res = get_a_byte( &next ) ) && next == curr )
count++;

/* Store the count and the value */

output_rle_data( count, curr );

/* Prepare for the next round with the byte we've just read in */

curr = next;
count = 1;
}
}

static int
get_a_byte( unsigned char * c )
{
return fread( c, 1, 1, in_fp );
}

static void
output_rle_data( unsigned char count,
unsigned char value )
{
fwrite( &count, 1, 1, out_fp );
fwrite( &value, 1, 1, out_fp );
}
--------------8<----------------------------------------

With this we have four functional units in the program - in main()
initialization is done (opening files etc.) and then the second
unit for doing the run-length encoding is called. That in turn
uses one unit for dealing with input and one for output.

Now, if you want to get more fancy and use buffers etc. you can
change the input and output functions to do that. For example,
we could change get_a_byte() to do instead

#define BUFFSIZE 5000

static int
get_a_byte( unsigned char * c )
{
static unsigned char buf[ BUFFSIZE ];
static size_t left_in_buf = 0,
next_in_buf;

if ( left_in_buf == 0 )
{
if ( ( left_in_buf = fread( buf, 1, BUFFSIZE, in_fp ) ) == 0 )
return 0;
next_in_buf = 0;
}

left_in_buf--;
*c = buf[ next_in_buf++ ];
return 1;
}

That way you read in a larger chunk of the input file at once
into a buffer and return bytes from that buffer. If the buffer
becomes empty you just read in one more chunk until the whole
file has been read in. All this is completely transparent to
the rest of the program and works with files of all sizes. The
function for doing the output can be modified in a similar way,
of course.
Regards, Jens
 
J

Jens Thoms Toerring

I didn't treat the case that there are 255 or more of the same
bytes in a row correctly, so two corrections must be made:
static void
rle_encode( void )
{
unsigned char count = 1,
curr,
next = 0;
int res;

/* Get the very first byte from the file */

res = get_a_byte( &curr );

/* Keep going while something could be read in */

while ( res != 0 )
{
/* Get a new byte from the file - stop if we already read 255 bytes
of the same value, if nothing could be read anymore or if the new
bytes value differs from the old one */
while ( count < 255 && ( res = get_a_byte( &next ) ) && next == curr )
count++;

/* Store the count and the value */

Insert here

if ( count > 0 )
output_rle_data( count, curr );

/* Prepare for the next round with the byte we've just read in */

curr = next;
count = 1;

Make that instead

count = count == 255 ? 0 : 1
Regards, Jens
 
B

Barry Schwarz

I didn't treat the case that there are 255 or more of the same
bytes in a row correctly, so two corrections must be made:

This no longer computes the letter distribution but counts consecutive
letters. Maybe you should first decide what the intent is.
 
J

Jens Thoms Toerring

Barry Schwarz said:
This no longer computes the letter distribution but counts consecutive
letters. Maybe you should first decide what the intent is.

Maxx wrote in his first post:

|| This technique reduces the size of the file by replacing the
|| original bytes with the repetition count and the byte to be repeated.
|| Ex: if the file to be compressed starts with the following sequence of
|| bytes::
|| 46 6F 6F 6F 2B 2B 9C 81 81 81 81 84 84 84 7A 7A 7A
||
|| The compressed file will contain the following bytes::
|| 01 46 03 6F 02 2B 01 9C 04 81 03 84 03 7A

That looks to me like he's jsut trying to count consecutive
characters, doesn't it?
Regards, Jens
 
M

Maxx

On Mon, 27 Dec 2010 07:41:39 -0800 (PST), Maxx


snip
I have rewritten the program using the guidelines that you've
provided, and it compiled well it didn't gave  any runtime errors, but

It still produces the wrong result.  Not all errors are caught by the
system.
the funny thing that happened is that the size of the resultant have
grown exactly twice. here is the program::

Since you write two bytes to your output file for every byte of your
input file, this is the only acceptable result.  What were you
expecting?




#include <stdio.h>
#include <stdlib.h>
#define MAXBUFF     5000
FILE *fp,*gp;
int get_a_char(unsigned char *);
void output_rle_data(unsigned char, unsigned char);
int main(int argc, char *argv[])
{
   int n=0,i,m,j,k;
   char name[20];
   unsigned char c,count,a[MAXBUFF],*p=a;
   sprintf(name,"%s.rle",argv[1]);

How do you insure that the user never provides a command line
parameter longer than 15 characters?  For that matter, how do you
insure that argv[1] exists and is not NULL?
   if(argc ==1)
   {

If you reach this point, the previous call to sprintf has already
invoked undefined behavior.
           fprintf(stderr,"Too few arguments");
           exit(1);

1 is not a portable return value from main.  Use EXIT_FAILURE which
is.
   }
   if((fp=fopen(*++argv,"rb")) ==NULL)
   {
           fprintf(stderr,"Can't open file",*argv);

What purpose do you think the third argument serves.  fprintf will
never look at it.  Did you mean to include a %s in your format string?
           exit(2);

2 is not portable either.
   if((gp=fopen(name,"wb"))==NULL)
   {
           fprintf(stderr,"Can't open file");

Wouldn't it be nice if you told the user which file could not be
opened?
           exit(2);
   }
   while((m=get_a_char(&c)) ==1)
   {
           *p++=c;

Due to an error in get_a_char, c is not assigned the character read
from the file.  c is not initialized or assigned to so its value is
indeterminate.  Your array a will be populated with garbage.

After you fix get_a_char, you still need to insure you do not overrun
the array a.
           ++n;
   }*p='\0';

To avoid confusion, this assignment should be on a new line.


   for(i=0;i<n;i++)
   {
           j=a,count=0;
           for(k=0;k<n;k++)
           {
                   if(j=a[k])
                           ++count;


Indenting consistently has one of the best cost/benefit ratios of all
style advice.  You would also be better off not using tabs in posted
code.

Why are you using an unsigned char to count occurrences.  Your array
holds 5000 elements.  Surely one value can occur more than 255 times.
           }
           output_rle_data(count,j);

You are aware that if a value appears multiple times in the array, you
will output its count each time.

main() returns an int and you should do so explicitly.
int get_a_char(unsigned char *c)
{
   return fread(&c, 1, 1, fp);

While this cannot produce a diagnostic because the first parameter to
fread is a void* and therefore any pointer argument is valid, you have
provided the wrong value.  You wanted fread to store the character
read from the file into the char whose address you passed in c.  That
would be the unsigned char c defined in main.  By using the same name
here, you managed to confuse yourself.  The argument you actually pass
to fread is the address of the local pointer in this function.  As a
result, fread will store the character read from the file in the first
byte of this pointer and not in the char defined in main.  Delete the
&.
void output_rle_data(unsigned char cn, unsigned char jn)
{
   fwrite(&cn,1,1,gp);
   fwrite(&jn,1,1,gp);
}



Yeah still got it wrong. Well the program did opposite of what i
expected it to do.,,my bad..

Ah its just a test so i set the buffer to a random value of 20.
What purpose do you think the third argument serves. fprintf will
never look at it. Did you mean to include a %s in your format string?
Yeah i did meant to include a %s in the format string. another
mistake.I used unsigned char cause to avoid all that negative values.
While this cannot produce a diagnostic because the first parameter to
fread is a void* and therefore any pointer argument is valid, you have
provided the wrong value. You wanted fread to store the character
read from the file into the char whose address you passed in c. That
would be the unsigned char c defined in main. By using the same name
here, you managed to confuse yourself. The argument you actually pass
to fread is the address of the local pointer in this function. As a
result, fread will store the character read from the file in the first
byte of this pointer and not in the char defined in main. Delete the
&.

Totally didn't saw it coming.Thanks for pointing this out
 
M

Maxx

Maxx said:
I have rewritten the program using the guidelines that you've
provided, and it compiled well it didn't gave  any runtime errors, but
the funny thing that happened is that the size of the resultant have
grown exactly twice. here is the program::
#include <stdio.h>
#include <stdlib.h>
#define MAXBUFF 5000
FILE *fp,*gp;
int get_a_char(unsigned char *);
void output_rle_data(unsigned char, unsigned char);
int main(int argc, char *argv[])
{
        int n=0,i,m,j,k;
        char name[20];

That may be much too short!
        unsigned char c,count,a[MAXBUFF],*p=a;
        sprintf(name,"%s.rle",argv[1]);

If the file name passed as the argument is has more than
15 characters thos will write past the end of the 'name'
array. Since you can't forsee how long the file name the
user inputs is going to be using a static buffer is rather
problematic - you would have to abbreiviate it if it's
too long (which might result in surprises for the user.
Better use a dynamically allocated buffer.
        if(argc ==1)
        {
                fprintf(stderr,"Too few arguments");
                exit(1);

Doing a return here will get you out also. And, while you're
free to return any integer value in principle, the only values
guaranteed to work everywere are EXIT_SUCESS and EXIT_FAILURE.
        }
        if((fp=fopen(*++argv,"rb")) ==NULL)

Why the strange "gymnastics" with the poor argv pointer?
        {
                fprintf(stderr,"Can't open file",*argv);
                exit(2);
        }
        if((gp=fopen(name,"wb"))==NULL)
        {
                fprintf(stderr,"Can't open file");
                exit(2);
        }
        while((m=get_a_char(&c)) ==1)
        {
                *p++=c;
                ++n;
        }*p='\0';

This is again very problematic. Consider what will happen if
the input file is longer than MAXBUFF-1: you will write past
the end of the 'a' array! And putting a '\0' in the last after
the last element is absolutely useless - this isn't a string.

Also, what I proposed was not to use any buffers in a first
version of the program. And if you use buffers it's not very
clever to use them in a way that restricts the possible size
of the files you can deal with. But if you do that you then
must check for longer files and find some way to deal with
the situation...
        for(i=0;i<n;i++)
        {
                j=a,count=0;
                for(k=0;k<n;k++)
                {
                        if(j=a[k])
                                ++count;
                }
                output_rle_data(count,j);
        }


Well, what you do here ins't run-length encoding at all. What
you do is counting for each position in the file how many bytes
exist in the whole file with the same value as at that position.
And then you write out the count and the value. Of course, the
resulting file will be twice as large as the input file. (And
the count may be wrong for a number of positions when the counter
overflowed).

Run-length encoding, as you described it yourself in your first
post, means something different: you count how many bytes of
the same value are in the file in the file, write that out and
then go on with the next set of bytes of the same value.

And then you're missing to return an int at the end;-)
}
int get_a_char(unsigned char *c)
{
        return fread(&c, 1, 1, fp);

Since you pass a pointer to the function this must be

         return fread( c, 1, 1, fp );

Note the removed '&' in front of 'c'.
}
void output_rle_data(unsigned char cn, unsigned char jn)
{
        fwrite(&cn,1,1,gp);
        fwrite(&jn,1,1,gp);
}

Here's a version ofhow I might do it:

--------------8<----------------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

static FILE *in_fp,
            *out_fp;

static void rle_encode( void );
static int get_a_byte( unsigned char * );
static void output_rle_data( unsigned char,
                             unsigned char );

int main( int    argc,
          char * argv[ ] )
{
    char *name;

    if ( argc < 2 )
    {
        fprintf( stderr, "Too few arguments\n" );
        return EXIT_FAILURE;
    }

    if ( ( in_fp = fopen( argv[ 1 ], "rb" ) ) == NULL )
    {

        fprintf( stderr, "Can't open input file '%s'\n", argv[ 1 ] );
        return EXIT_FAILURE;
    }

    if ( ( name = malloc( strlen( argv[ 1 ] ) + 5 ) ) == NULL )
    {
        fprintf( stderr, "Running out of memory\n" );
        fclose( in_fp );
        return EXIT_FAILURE;
    }

    sprintf( name, "%s.rle", argv[ 1 ] );

    if ( ( out_fp = fopen( name, "wb" ) ) == NULL )
    {
        fprintf( stderr,"Can't open output file '%s'\n", name );
        free( name );
        fclose( in_fp );
        return EXIT_FAILURE;
    }

    free( name );

    rle_encode( );

    fclose( out_fp );
    fclose( in_fp );
    return EXIT_SUCCESS;

}

static void
rle_encode( void )
{
    unsigned char count = 1,
                  curr,
                  next = 0;
    int res;

    /* Get the very first byte from the file */

    res = get_a_byte( &curr );

    /* Keep going while something could be read in */

    while ( res != 0 )
    {
        /* Get a new byte from the file - stop if we already read 255 bytes
           of the same value, if nothing could be read anymore or if the new
           bytes value differs from the old one */

        while ( count < 255 && ( res = get_a_byte( &next ) ) && next == curr )
            count++;

        /* Store the count and the value */

        output_rle_data( count, curr );

        /* Prepare for the next round with the byte we've just read in */

        curr = next;
        count = 1;
    }

}

static int
get_a_byte( unsigned char * c )
{
    return fread( c, 1, 1, in_fp );

}

static void
output_rle_data( unsigned char count,
                 unsigned char value )
{
    fwrite( &count, 1, 1, out_fp );
    fwrite( &value, 1, 1, out_fp );}

--------------8<----------------------------------------

With this we have four functional units in the program - in main()
initialization is done (opening files etc.) and then the second
unit for doing the run-length encoding is called. That in turn
uses one unit for dealing with input and one for output.

Now, if you want to get more fancy and use buffers etc. you can
change the input and output functions to do that. For example,
we could change get_a_byte() to do instead

#define BUFFSIZE 5000

static int
get_a_byte( unsigned char * c )
{
    static unsigned char buf[ BUFFSIZE ];
    static size_t left_in_buf = 0,
                  next_in_buf;

    if ( left_in_buf == 0 )
    {
        if ( ( left_in_buf = fread( buf, 1, BUFFSIZE, in_fp ) ) == 0 )
            return 0;
        next_in_buf = 0;
    }

    left_in_buf--;
    *c = buf[ next_in_buf++ ];
    return 1;

}

That way you read in a larger chunk of the input file at once
into a buffer and return bytes from that buffer. If the buffer
becomes empty you just read in one more chunk until the whole
file has been read in. All this is completely transparent to
the rest of the program and works with files of all sizes. The
function for doing the output can be modified in a similar way,
of course.
                            Regards, Jens


As this program was just a test so i've used a random value of 20 as
my file name buffer, very immature of me.Ok now i've realized the
problems with the buffers, they always tend to bug me.
What i couldn't get my head around was the main alogrithm of the
program, previously i thought my method would give me the correct
result, i also did some trials with a few values and all that seem
fair, but i was completely missing the bigger picture, which i why my
program failed every time.
And thanks a lot for your program, it really cleared all my
doubts.Thanks
 
M

Michael Press

James Dow Allen said:
        if(fp=fopen(*++argv,"rb")==NULL) /* bug */
[more code whose functionality, or lack thereof,
is irrelevant]

My comment is intended not just for Maxx, but for
Arnuld and a few others who post here.

I've been programming for quite a while now, and make
relatively few mistakes, yet I *STILL* will compile
and run code before it's finished, just to do piecemeal
verification. Perhaps I should be embarrassed to

I will compile with cc -c _before_ it can run.
admit it, but sometimes I even have something like
printf("Last arg is *%s*\n", argv[argc-1]);
just to make sure I've not gone off-by-one somewhere.
Yet we see complicated programs posted with the
first line wrong! :)

Long ago I worked in a 24-hour turnaround environment:
code as much as you can accurately in a day, and
study the coredump because *you only get one
compile-and-go per day!* Those days are gone now.
Use the compiler interactively and let it
be your friend!

Uh huh. I get worried when there
are no compile errors or warnings.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,733
Messages
2,569,440
Members
44,829
Latest member
PIXThurman

Latest Threads

Top