Feedback på stil til C-newbie

Som nævnt i en anden kategori, har jeg brug for at beregne en id ud fra en mp3-fil. Kravet er at id'en er den samme, selv om filen bliver flyttet, omdøbt eller får fjernet/tilføjet/ændret id3-tags.

Jeg har nu fundet en algoritme, som ser ud til at fungere. Jeg har forsøgt at implementere den i C++, og det ser også ud til at fungere. Men da jeg er en C(++)-newbie vil jeg gerne have feedback på min kode.

- Er der noget som inviterer til problemer?
- Er der noget som gøres bedre/nemmere/mere effektivt på en anden måde?
- Er der noget som ser ud til at være direkte fejl?

Det er en del af et program til håndtering af musik-samlinger. Jeg har planer om at udgive programmet under GPL.

Algoritmen:
Først tjekkes om det overhovedet er en mp3-fil. Dette gøres ved at se om de tre første bytes er "ID3", eller de 12 første bits er sat. Er en af delene opfyldt, formodes det at være en mp3-fil.

ID3-tags er 128 bytes tilføjet i slutningen af mp3-filen. De 128 bytes starter med strengen "TAG". Dvs. starter de sidste 128 bytes med "TAG", så skal disse 128 bytes ignoreres.

Der beregnes så en id ud fra den del af filen, der ikke er id3-tags. Id'en er "md5-agtig". Dvs. den består af 16 bytes, og vises som en streng med 32 tegn, som er tal fra 0-9 og bogstaverne A-F.
De 16 bytes fyldes først med de 16 første bytes fra filen, fra venstre (index 0) mod højre (index 15). Hele filen (undtagen id3) køres så igennem, hvor der hele tiden fyldes  på fra venstre mod højre. Ligesom når man skriver igen og igen på samme linje, på et stykke papir.
Data kombineres med eksisterende data via XOR.

Hvis man ændrer så meget som én vilkårlig bit i filen (stadig id3 undtaget), vil id'en ændre sig. Sandsynligheden for at id'en for to forskellige filer er ens, er forhåbentlig noget i stil med md5.

Jeg har valgt ikke at bruge md5 af flere årsager. Det kan koges ned til at dette er lettere for mig (tilsyneladende uden at miste noget væsentligt), og så bruger det vist også væsentligt mindre CPU-kraft. (Ret væsentligt når man lige skal køre 3 gb mp3'er igennem.)
Eneste ulempe jeg lige kan komme på er, at det er nemt at skabe en bestemt id.
Men det er da muligt at jeg på et tidspunkt vælger at bruge md5 i stedet for dette simple XOR.


Koden:
/*
* boolean file2id(char id[], char file[])
*
* Generates an id for an mp3. The id should not change if file is moved,
* renamed, or have id3-tags added, removed or changed.
*
* Parameters:
* char id[]: The id will be saved in this var, as a 32 bytes string (plus a
* null byte). The string only have digits and letters from A to F.
* char file[]: Path and filename to an mp3 file.
*
* Returns.
* Boolean: True on success, false if id could not be generated. Reasons for
* failure includes file not readable and file not recognized as MP3.
*
* Note: printf messages is only for debug purposes, will be removed later.
* (FIXME)
*/

bool file2id(char id[], char file[]) {
  FILE *fp; 
  unsigned char buf[3];
  long bytestoread;
  unsigned char sum[17];
  char dec2hex[] = {'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F'};

  printf("Making ID from: %s\n", file);
  fp = fopen(file, "rb");
  if (!fp) {
    printf("  Couldn't open file for reading\n");
    return false;
  }
 
  fread(buf, 3, 1, fp);
  if (buf[0]!='I' || buf[1]!='D' || buf[2]!='3') {
    // File does not start with "ID3".
    buf[0] &= 0xff;
    buf[1] &= 0xf0;                                                       
    if(buf[0]!=0xff  || buf[1]!=0xf0){
      // The first 12 bits isn't all set, it's not an mp3-file.
      printf("  Is not an MP3-file\n");
      fclose(fp);
      return false;
    }
  }

  if (fseek(fp, -128, SEEK_END) != 0) {
    printf("  Could not seek to 128 bytes before end.\n");
    fclose(fp);
    return false;
  }

  fread(buf, 1, 3, fp);
  if (buf[0]!='T' || buf[1]!='A' || buf[2]!='G') {
    // The last 128 bytes does not start with "TAG", no ID3-tags.
    printf("  File does NOT have ID3 tags, making checksum of entire file.\n");
    fseek(fp, 0, SEEK_END);
  } else {
    printf("  File DOES have ID3 tags, don't use last 128 bytes in checksum.\n");
    fseek(fp, -128, SEEK_END);
  }

  bytestoread = ftell(fp); /* If there's id3-tags, this will be the filesize
                  minus the id3-tags. This way, added, removed or
                  changed ID3-tags will not be seen. */

  fseek(fp, 0, SEEK_SET); // Go to start of file.

  for (int i=0; i<bytestoread; i++) {
    sum[i%16] ^= fgetc(fp); /* FIXME: Errorhandling. If something changes, or
                  parts of file is unreadable, wrong id might be
                  generated with no way of knowing that something
                  is wrong. */
  }

  // sum should now be 16 bytes, 16 numbers between 0 and 255.

  // Let's convert those 16 bytes to a 32 chars, "md5-style".

  for (int i=0; i<16; i++) {
    id[i*2] = dec2hex[sum[i]/16];
    id[i*2+1] = dec2hex[sum[i]%16];
  }

  id[32] = '\0'; // Makes it a string with a null in the end.

  return true;
}