7

I have completed the pset, and am happy with my code and how it works. But had some design questions so that I know for future programs what and why to use.

I understand that it does not really matter for the scale of this program. I will not post the entire code here since that is against the policy but can send a pastebin in private if you'd like.

No lacanian or freudian responses re:sexy code, it is admittedly a linkbait title.

buffer variables

What are the proper ways of buffering partial contents of a file in a way that one can compare the contents? In my Tarzan C, I used the following because char would store the ASCII codes for the hexadecimal digits and I wasn't sure if I could compare those. Should I have used plain int? How does one compare hexadecimal strings with that? I used uint8_t because that was storing what I was looking for when I printed in gdb.

        uint8_t buffer[511];

comparing file contents

In comparing the beginning of the buffer with (0xff 0xd8 0xff 0xe0 || 0xff 0xd8 0xff 0xe0), I employed the good old randomly accessible char array and referenced characters by position individually. Is there a more elegant way of doing this? I know you can use memcmp(string.h) but that would require the declarations of two more arrays and would look just as, if not more convoluted, given that the program only looks for two non changing hex strings. Here is what I used. Should I have typecast or something instead?

        if ( buffer[0] == 0xFF && buffer[1] == 0xD8 && buffer[2] == 0xFF && ( buffer[3] == 0xE0 || buffer[3] == 0xE1 ) )

pointer declarations and sprintf

answer

As per Kareem's answer, I had just carried over an earlier troubleshooting attempt of an earlier design problem, thinking that sprintf was a function and therefore could not accept variables, the following is perfectly fine

int file_counter = 0;
sprintf (outptr_name,"%.3d.jpg", pfile_counter++);

question

To keep track of the file number, I used a pointer. Two questions here, should I have just malloc() in declaring the int *pfile_counter instead of declaring an int first and pointing to the address of that int? Or should I have just used a global variable instead of a pointer?

int file_counter = 0;
int *pfile_counter = &file_counter;

Here is how I am using the counter, is the sprintf redundant? could I have used lower level code to set the file name?

sprintf (outptr_name,"%.3d.jpg", (*pfile_counter)++);

feof order of operation

Finally, I run the main reading/writing bits in a forever loop that breaks out when fread sets feof. It does not matter particularly for this pset because of the generous padding at the end of the last file but I'm a bit confused about the order of things in utilizing feof.

When fread() reads the end of the file, and say it is not 512 bytes but instead 500, and happens to contain data that I want to writeout, does it read both the 500 bytes and the feof, or does it read the 500 bytes, and returns feof the next time I run fread()?

here is how I declared fread(), I found that if you don't ask for 512 elements but one element of size 512, it will return 0 even if you have say 500 bytes left before EOF

    fread(&buffer, sizeof(uint8_t), 512, file);

In that, should I be designing the loop so that it iterates one last time after detecting feof, writing out the buffer that reached the end of the file? Or should the loop quit before writing the last read buffer because it only contains feof? From what I gather, it would return the number of data elements, so 512 until the end, 500 at the end (and again let's say the last block is 500 bytes though it can't be in FAT.), and 0 once you try again, which sets feof(). Diagrammed below is what I am guessing to happen at the end of the file, including the last full block.

reads 512 elements | reads 500 as there is no more | reads no elements
returns 512        | returns 500                   | returns 0; sets feof()

people seem to keep referring to the cprogramming.com article but that example only fails because he/she is reading first, and processing the buffer in the rest of the loop. One could easily employ fread at the end as below, and skip fwrite if reading has not begun yet (ie.first iteration)

With the assumption above, the following pseudocode would work in itself because with the read and write reversed, buffer would not be processed once feof is true ( i dismissed the rest for brevity, assume we are handling openings and closing elsewhere )

while(!feof){fwrite(output); fread(input;}

thank you!

Phew, that was a bunch of questions.. Thank you very much. I exhausted my capacity to understand the books on these, I only started to course 8-9 days ago and never saw C before.

5

char would store the ASCII codes for the hexadecimal digits

Well, not really! If you do something like

unsigned char c = 0xff; // hex

this won't store the ASCII value of 255(dec) because 255(dec) is not actually a char (like 'A' or '9'). It's an integer value and so are ASCII values (i.e., they're also integers).

The thing is that variables of the regular char type are typically interpreted as signed chars. This means that they can only store up to 127(dec) and this could cause problems because out of bounds values would wrap around. For example,

char d = 128; // stores -128(dec)
int i = 128; // stores 128(dec)

if (d == i)
    printf("they're equal!\n");
else
    printf("they're NOT equal!\n");

Output:

they're NOT equal!

So in this situation, unsigned char and uint8_t can be used interchangeably. If you used an int, this would probably be waste of space (since it's 4 times bigger than what we really need).

I also want to emphasize that the values 0xff, 0xd8, 0xe0, and 0xe1 are not strings. They are integer values.

Is there a more elegant way of doing this?

I don't think so. You could just break the long condition over multiple lines so that it looks nicer.

Should I have typecast or something instead?

Not necessary here because values of type unsigned char or uint8_t are implicitly (silently) promoted to an int (the default data type for 0xff and the others) then compared against them in this situation.

should I have just malloc() in declaring the int *pfile_counter instead of declaring an int first and pointing to the address of that int?

These are two different things. If you used malloc you would be allocating memory on the heap which is not really necessary in this situation. In fact, I'm not really sure why you even need to use a pointer to your counter. You could just pass file_counter++ to your sprintf directly since %d expects "an int".

Or should I have just used a global variable instead?

Well, that wouldn't probably be the best design. Using global variables is generally discouraged unless you really need to.

does it read both the 500 bytes and the feof, or does it read the 500 bytes, and returns feof the next time I run fread()?

The function feof returns true in case an attempt to read data is made after the EOF was reached. So basically if a file contains only 500 bytes of data and fread tried to read 512 bytes of data, the data in the 500 bytes will be read into the buffer, the remaining 12 bytes will be filled with garbage values and if you called feof at that moment, it would return true.

If however fread tried to read only 500 bytes, it wouldn't set the EOF and therefore, feof would return false.

In the specification, the raw file consists of a number of 512 blocks. This means that no matter how big the raw file is, 512 will divide its size evenly and since fread reads 512 bytes at a time, there will eventually be a point where fread reads the last block, but not the EOF and that leaves you with an additional iteration.

I found that if you don't ask for 512 elements but one element of size 512, it will return 0 even if you have say 500 bytes left before EOF

If I get that well, you're asking that if you did something like

fread(buffer, 512, 1, stream);

on a stream that only contains 500 bytes, fread would return 0. That is true because fread returns the number of elements of size 512 bytes, in this example, that were read successfully. Since only 500 were read, this means that 0 elements of 512 were read successfully.

One could easily employ fread at the end as below, and skip fwrite if reading has not begun yet (ie.first iteration)

That would probably require you to write some code inside the loop that checks whether this is the first iteration of the loop and this condition will be unnecessarily checked in each iteration of the loop. I think the solution that was in the article is ideal except that, in this case, you probably should read n elements of size 1 byte to ensure that when 0 is returned this means that no data was read at all.

| improve this answer | |
  • On the pointer value, you are of course right. Very early on in writing the program I must have had a nesting issue where incrementing the value was not working as the software just overwrote 000.jpg, and I had assumed in my beginners wisdom that sprintf() was a function and would copy variables instead of taking them. – freshsisyphus Feb 24 '15 at 12:29
  • on the feof, So in a file that does not end cleanly at the end of a block, say, I fread 512 bytes into my array but there is only 500, would the last 12 bytes of my array just be the values from the previous fread that filled my array with 512? Or would fread go beyond the file and read whatever is in memory beyond it? Given all this then, a proper implementation that accomodates truncated blocks would be to use the returned integer by fread() as the third argument of fwrite() to write only that many bytes and leave the garbage in the array right? – freshsisyphus Feb 24 '15 at 12:47
  • on the char > char d = 128; // stores -128(dec) you mean it would wrap around to -127 because there 128 is beyond size, so it stores -127 right? – freshsisyphus Feb 24 '15 at 12:48
  • @freshsisyphus For your second comment, not really. As stated in the answer, in this case, the last 12 bytes are filled with garbage values (values that could be anything). For your third comment, no actually the least value that a signed char can store is -128 not -127. A char is typically a byte long. A variable of size 1 byte can store 2^8 = 256 values. Since it's signed (it's capable of representing positive and negative) these values are from 0 to 127 and from -1 to -128. Recall that there are no positive and negative zeros. – Kareem Feb 24 '15 at 13:48
  • The first comment was simply that I had made it into a pointer in an earlier design of my program, because I had a nesting problem where I was incrementing the value at a wrong point and thought sprintf was not getting it. Obviously, the problem was elsewhere as I now use sprintf (outptr_name,"%.3d.jpg", pfile_counter++; without problems – freshsisyphus Feb 24 '15 at 13:54

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .