Overwriting an ID3v2 tag in C using fwrite()

Question

For some reason, when I call fwrite(), it doesn't overwrite the file pointed to by the pointer. When I run the program, it displays that the tag of the file was replaced by the inputted tag specified by the user. After I check the file using a separate code to see the current tags, the tag wasn't replaced at all. I believe there is something wrong with how I used fwrite and into thinking that it would really overwrite the file. The tag in this case is the title. Here is the code:

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#include "title.h"

char title[30];
char title1[30];

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

FILE *fPtrs;

    // print the current tag
fPtrs = fopen(argv[1],"r+b");
fseek(fPtrs,-125,SEEK_END);
fread(title1,1,30,fPtrs);
strncpy(title,title1,30);
printf("%s\n",title);
fclose(fPtrs);

    // call to overwrite the current tag
fPtrs = fopen(argv[1],"w+b");
title_tag(fPtrs,argv[2]);   
fclose(fPtrs);

    // print the tag of the should be overwritten file
fPtrs = fopen(argv[1],"r+b");
fseek(fPtrs,-125,SEEK_END);
fread(title1,1,30,fPtrs);
strncpy(title,title1,30);
printf("%s\n",title);
fclose(fPtrs);

return 0;
}

#include<stdio.h>
void title_tag(FILE* fName, char title_s[]){

fseek(fName,-125,SEEK_END);
fwrite(title_s,1,30,fName);

}

This is a project I'm working on in the university and we were told that we were not allowed to use the id3lib -.-

`fseek`, `fread` and `fwrite` all return errors. Checking these might help point you towards the problem. — simonc, Oct 10 '13 at 17:30

score 1 · Answer 1 · answered Oct 14 '13 at 19:17

You aren't doing any input validation. Your program depends on there being two command-line arguments, but you aren't testing for argc == 3 or anything similar.
There's no error checking. After blindly assuming that the first command-line argument was a correctly specified filename, you aren't checking the return value of the fopen() call to verify that the call succeeded. You also aren't doing any checking to verify that the file you've opened has an ID3v1 tag at the end, you're simply assuming it is present.
When you call fread(), you're not making sure that what you read was a valid string, but you're then proceeding to treat it as one. (You're reading in a 30-byte field, but it doesn't appear to me that it's required to be a null-terminated 30-byte field. What happens if the track title is 30 bytes long?
Why are you copying the title from title into title1? Totally leaving aside that you haven't ensured that it's null-terminated, and therefore should be either null-terminating it or using memcpy(), the only thing you're doing with the copy is passing it to printf(). What did copying it gain you?
You shouldn't be closing/reopening/closing/reopening the file. The first time you open it, you're using mode "r+b", which opens the file for reading and writing, so all you needed to do was that fseek(). (And, of course, sanity-checking to see if you can update an existing ID3v1 tag or if you need to append one.)
You really shouldn't be closing/reopening/closing/reopening the file. The open mode "w+b", which you're using when you unnecessarily close/reopen the file to update it, is documented to truncate the file to zero length if the file already exists, or create a new file otherwise.
What you're passing to your title_tag() function.
- Because of what you're doing with it, the second argument to title_tag() needs to be a block of at least 30 bytes of storage. You're passing it argv[2], which is not guaranteed to be such. You'll need to copy argv[2] into your own storage, and pass that instead.
- You've written the title_tag() function such that someone might mistakenly think the second argument in the function is an array. However, due to the mechanics of the C language, within the context of the function the argument is simply a pointer-to-char. (You're allowed to write it as char title_s[], but in C, if you pass an array as an argument to a function, what the function actually gets is a pointer to the first element of the array. For most purposes, this makes no functional difference.)

There. Hope that helps.

score 0 · Answer 2 · answered Oct 10 '13 at 17:32

0

highly recommend using the id3lib. will allow you better control over the manipulation of id3 tags rather than just trying to write to the file raw

http://id3lib.sourceforge.net/

answered Oct 10 '13 at 17:32

Dani

1,220
7
22

If you're going to recommend a (L)GPL library, it'd be worth also explaining what restrictions this imposes. (i.e. releasing a product entitles users to either full source code or an application structure that allows id3lib to be independently updated) – simonc Oct 10 '13 at 17:37
I guess, if you say so – Dani Oct 10 '13 at 21:21

Overwriting an ID3v2 tag in C using fwrite()

2 Answers2

You really shouldn't be closing/reopening/closing/reopening the file. The open mode `"w+b"`, which you're using when you unnecessarily close/reopen the file to update it, is documented to truncate the file to zero length if the file already exists, or create a new file otherwise.

Overwriting an ID3v2 tag in C using fwrite()

2 Answers2

You really shouldn't be closing/reopening/closing/reopening the file. The open mode "w+b", which you're using when you unnecessarily close/reopen the file to update it, is documented to truncate the file to zero length if the file already exists, or create a new file otherwise.

You really shouldn't be closing/reopening/closing/reopening the file. The open mode `"w+b"`, which you're using when you unnecessarily close/reopen the file to update it, is documented to truncate the file to zero length if the file already exists, or create a new file otherwise.