Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@rstemmer
rstemmer committed Aug 11, 2018
1 parent d5bbf96 commit e97ffe2
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 61 deletions.
7 changes: 2 additions & 5 deletions encoding.c
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
//#include <string.h>
//#include <unistd.h>
//#include <endian.h>
#include <id3v2.h>
#include <id3v2frame.h>
#include <iconv.h>
Expand All @@ -13,7 +10,7 @@
#include <printhex.h>
#endif

int Transcode(const char *from, const char *to, void *input, size_t inputbytelimit, void *output, size_t outputbytelimit, size_t *actualsize)
int Transcode(const char *from, const char *to, const void *input, size_t inputbytelimit, void *output, size_t outputbytelimit, size_t *actualsize)
{
iconv_t cd;
size_t retval;
Expand Down Expand Up @@ -103,7 +100,7 @@ int Decode(unsigned char id3v2encoding, void *rawdata, size_t rawdatasize, char
}


int Encode(unsigned char id3v2encoding, char *utf8text, size_t textlength, void *rawdata, size_t rawdatasizelimit, size_t *actualsize)
int Encode(unsigned char id3v2encoding, const char *utf8text, size_t textlength, void *rawdata, size_t rawdatasizelimit, size_t *actualsize)
{
// prepare transcoding
const char *code;
Expand Down
4 changes: 2 additions & 2 deletions encoding.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
* Terminating character will be "transcoded" as well
* actualsize is the number of bytes written into the output buffer
*/
int Transcode(const char *from, const char *to, void *input, size_t inputbytelimit, void *output, size_t outputbytelimit, size_t *actualsize);
int Transcode(const char *from, const char *to, const void *input, size_t inputbytelimit, void *output, size_t outputbytelimit, size_t *actualsize);

/*
* returns a 0-terminated utf-8 string in utf8text variable
* The actual size of bytes in the output buffer gets written to actualsize when not NULL
* Decoding will be done for rawdatasize bytes. Termination sequences will be ignored
*/
int Decode(unsigned char id3v2encoding, void *rawdata, size_t rawdatasize, char *utf8text, size_t textlengthlimit, size_t *actualsize);
int Encode(unsigned char id3v2encoding, char *utf8text, size_t textlength, void *rawdata, size_t rawdatasizelimit, size_t *actualsize);
int Encode(unsigned char id3v2encoding, const char *utf8text, size_t textlength, void *rawdata, size_t rawdatasizelimit, size_t *actualsize);
#endif


Expand Down
21 changes: 10 additions & 11 deletions id3v2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ int ID3V2_Open(ID3V2 **id3v2, const char *path, bool createtag)
}

// Copy path
int pathlength = strlen(path) + 1; // don't forget the terminating \0
size_t pathlength = strlen(path) + 1; // don't forget the terminating \0
id3->path = (char*) malloc(sizeof(char)*pathlength);
if(id3->path == NULL)
{
Expand All @@ -85,10 +85,10 @@ int ID3V2_Open(ID3V2 **id3v2, const char *path, bool createtag)
}

// Read Header
fread(&id3->header.ID, 1, 3, id3->file);
fread(&id3->header.version_major, 1, 1, id3->file);
fread(&id3->header.version_minor, 1, 1, id3->file);
fread(&id3->header.flags, 1, 1, id3->file);
fread(&id3->header.ID, 1, 3, id3->file);
fread(&id3->header.version_major, 1, 1, id3->file);
fread(&id3->header.version_minor, 1, 1, id3->file);
fread(&id3->header.flags, 1, 1, id3->file);

// the header size is stored as big endian. This must be converted to little endian.
// beside this, it has 7 bit per byte...
Expand All @@ -110,7 +110,6 @@ int ID3V2_Open(ID3V2 **id3v2, const char *path, bool createtag)
if(createtag)
printf("\e[1;30m\tCreating new Tag if 0x%02X == 0xFF && 0x%02X ∈ {0xFD,0xFB,0xFA,0xF3}\n", id3->header.ID[0], id3->header.ID[1]);
}
//printf("\e[0m0x%08X -> %08X\n", bigendian, id3->header.size);

// check if I support this ID3 thing. If not, create one if allowed
if(id3->header.ID[0] != 'I' || id3->header.ID[1] != 'D' || id3->header.ID[2] != '3')
Expand Down Expand Up @@ -281,7 +280,7 @@ int ID3V2_Open(ID3V2 **id3v2, const char *path, bool createtag)
// Check if valid, if not, try to rescue the file by assuming that the headers size value is invalid
if((char)data != 0x00)
{
fprintf(stderr, "\e[1;31mBad padding byte found at offset %i (file pos. %li) - expected at offset %i.\n",
fprintf(stderr, "Bad padding byte found at offset %i (file pos. %li) - expected at offset %i.\n",
offset, ftell(id3->file) - 1, id3->header.origsize);
fprintf(stderr, "\tInvalid byte: 0x%X\n", ((unsigned)data)&0x00FF);
fprintf(stderr, "\tI\'ll try to fix it. If it doesn\'t work, nothing will become worse.\n");
Expand All @@ -304,7 +303,7 @@ int ID3V2_Open(ID3V2 **id3v2, const char *path, bool createtag)
&& magicword != 0xFDFF
&& magicword != 0xF3FF) // keep LE in mind
{
fprintf(stderr, "\e[1;31mMagic mp3-ID not found at expected offset %i (file pos. %li)!\n",
fprintf(stderr, "Magic mp3-ID not found at expected offset %i (file pos. %li)!\n",
offset, ftell(id3->file) - 1);
fprintf(stderr, "\tInvalid ID: 0x%4X\n", magicword&0x0FFFF);
fprintf(stderr, "\tAt file pos. %li\n", ftell(id3->file));
Expand Down Expand Up @@ -501,7 +500,7 @@ int ID3V2_Close(ID3V2 *id3v2, const char *altpath, bool removetag)

//////////////////////////////////////////////////////////////////////////////

static ID3V2_FRAME *GetFrameById(ID3V2 *id3v2, const unsigned int ID)
static ID3V2_FRAME *GetFrameById(const ID3V2 *id3v2, unsigned int ID)
{
ID3V2_FRAME *frame;
frame = id3v2->framelist;
Expand Down Expand Up @@ -597,7 +596,7 @@ int ID3V2_RemoveAllFrames(ID3V2 *id3v2)

//////////////////////////////////////////////////////////////////////////////

int ID3V2_GetFrame(ID3V2 *id3v2, const unsigned int ID, unsigned int *size, void **data)
int ID3V2_GetFrame(const ID3V2 *id3v2, unsigned int ID, size_t *size, void **data)
{
#ifdef DEBUG
printf("\e[1;37mGet frame: ");
Expand Down Expand Up @@ -630,7 +629,7 @@ int ID3V2_GetFrame(ID3V2 *id3v2, const unsigned int ID, unsigned int *size, void

//////////////////////////////////////////////////////////////////////////////

int ID3V2_SetFrame(ID3V2 *id3v2, const unsigned int ID, unsigned int size, void *data)
int ID3V2_SetFrame(ID3V2 *id3v2, unsigned int ID, size_t size, const void *data)
{
#ifdef DEBUG
printf("\e[1;37mSet frame: ");
Expand Down
4 changes: 2 additions & 2 deletions id3v2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ int ID3V2_RemoveAllFrames(ID3V2 *id3v2);
* Remove all frames
*/

int ID3V2_GetFrame(ID3V2 *id3v2, const unsigned int ID, unsigned int *size, void **data);
int ID3V2_GetFrame(const ID3V2 *id3v2, unsigned int ID, size_t *size, void **data);
/*
* the size of the frame gets returned by writing to the size pointer
* for data, new memory gets allocated and a copy of the data will be created.
Expand All @@ -127,7 +127,7 @@ int ID3V2_GetFrame(ID3V2 *id3v2, const unsigned int ID, unsigned int *size, void
* # *title = NULL;
*/

int ID3V2_SetFrame(ID3V2 *id3v2, const unsigned int ID, unsigned int size, void *data);
int ID3V2_SetFrame(ID3V2 *id3v2, unsigned int ID, size_t size, const void *data);
/*
* # error = ID3V2_SetFrame(id3v2, 'TIT2', size, title);
*/
Expand Down
24 changes: 12 additions & 12 deletions id3v2frame.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@
#include <printhex.h>
#endif

int ID3V2_GetTextFrame(ID3V2 *id3v2, const unsigned int ID, char *utf8text, size_t bufferlimit)
int ID3V2_GetTextFrame(const ID3V2 *id3v2, unsigned int ID, char *utf8text, size_t bufferlimit)
{
int error;

// Get raw data
unsigned int framesize;
size_t framesize;
unsigned char *framedata;
error = ID3V2_GetFrame(id3v2, ID, &framesize, (void**)&framedata);
if(error)
Expand Down Expand Up @@ -44,16 +44,16 @@ int ID3V2_GetTextFrame(ID3V2 *id3v2, const unsigned int ID, char *utf8text, size

//////////////////////////////////////////////////////////////////////////////

int ID3V2_SetTextFrame(ID3V2 *id3v2, const unsigned int ID, char *utf8text, unsigned char encoding)
int ID3V2_SetTextFrame(ID3V2 *id3v2, unsigned int ID, const char *utf8text, unsigned char encoding)
{
if(utf8text == NULL)
return ID3V2ERROR_NOERROR;

int error;
size_t textlength = strlen(utf8text) + 1; // count '\0' as well
int error;
size_t textlength = strlen(utf8text) + 1; // count '\0' as well
void *rawtext;
size_t rawtextsize;
size_t textbufferlimit = textlength * 4; // 4 time the uft-8 encoded size is enough
size_t textbufferlimit = textlength * 4; // 4 time the uft-8 encoded size is enough

rawtext = malloc(textlength * 4);
if(rawtext == NULL)
Expand Down Expand Up @@ -111,13 +111,13 @@ int ID3V2_SetTextFrame(ID3V2 *id3v2, const unsigned int ID, char *utf8text, unsi

//////////////////////////////////////////////////////////////////////////////

int ID3V2_GetPictureFrame(ID3V2 *id3v2, const unsigned char pictype,
int ID3V2_GetPictureFrame(const ID3V2 *id3v2, unsigned char pictype,
char **mimetype, char **description, void **picture, size_t *picsize)
{
int error;

// Get raw data
unsigned int rawsize;
size_t rawsize;
int rawoffset = 0; // tracing offset through the process to know where we are in the frame
unsigned char *rawdata;
error = ID3V2_GetFrame(id3v2, 'APIC', &rawsize, (void**)&rawdata);
Expand Down Expand Up @@ -256,9 +256,9 @@ int ID3V2_GetPictureFrame(ID3V2 *id3v2, const unsigned char pictype,

//////////////////////////////////////////////////////////////////////////////

int ID3V2_SetPictureFrame(ID3V2 *id3v2, const unsigned char pictype,
int ID3V2_SetPictureFrame(ID3V2 *id3v2, unsigned char pictype,
const char *mimetype, const char *description, unsigned char encoding,
void *picture, size_t picsize)
const void *picture, size_t picsize)
{
int error;

Expand Down Expand Up @@ -303,7 +303,7 @@ int ID3V2_SetPictureFrame(ID3V2 *id3v2, const unsigned char pictype,
}

// Build frame
unsigned int framesize = 0;
size_t framesize = 0;
unsigned char *framedata = NULL;
unsigned int frameoffset = 0;

Expand Down Expand Up @@ -331,7 +331,7 @@ int ID3V2_SetPictureFrame(ID3V2 *id3v2, const unsigned char pictype,
memcpy(framedata+frameoffset, picture, picsize); // picture data

#ifdef DEBUG
unsigned int mimesize = strlen(mimetype);
size_t mimesize = strlen(mimetype);
printhex(framedata, 128, 16,
0, "\e[1;36m", // encoding
1, "\e[1;35m", // mime type
Expand Down
10 changes: 5 additions & 5 deletions id3v2frame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* after getting the addressed Frame.
* bufferlimit: is the size of the utf8text buffer in bytes
*/
int ID3V2_GetTextFrame(ID3V2 *id3v2, const unsigned int ID, char *utf8text, size_t bufferlimit);
int ID3V2_GetTextFrame(const ID3V2 *id3v2, unsigned int ID, char *utf8text, size_t bufferlimit);

/*
* ID: 4 Byte Frame ID
Expand All @@ -24,20 +24,20 @@ int ID3V2_GetTextFrame(ID3V2 *id3v2, const unsigned int ID, char *utf8text, size
*
* !! When an encoding is used that is not supported by the given ID3 version, the version gets updated in the ID3V2 struct
*/
int ID3V2_SetTextFrame(ID3V2 *id3v2, const unsigned int ID, char *utf8text, unsigned char encoding);
int ID3V2_SetTextFrame(ID3V2 *id3v2, unsigned int ID, const char *utf8text, unsigned char encoding);

#define ID3V2_MAXPICTUREDESCRIPTIONSIZE ((64+1)*2) /*max 64 Chars + termination character UTF-16 encoded*/
/*
* mimetype, description and picture memory must be feed by the caller when not NULL
* description: can be NULL
* encoding: The ID3V2TEXTENCODING to use to encode the text
*/
int ID3V2_GetPictureFrame(ID3V2 *id3v2, const unsigned char pictype,
int ID3V2_GetPictureFrame(const ID3V2 *id3v2, unsigned char pictype,
char **mimetype, char **description, void **picture, size_t *picsize);

int ID3V2_SetPictureFrame(ID3V2 *id3v2, const unsigned char pictype,
int ID3V2_SetPictureFrame(ID3V2 *id3v2, unsigned char pictype,
const char *mimetype, const char *description, unsigned char encoding,
void *picture, size_t picsize);
const void *picture, size_t picsize);
#endif


Expand Down
36 changes: 18 additions & 18 deletions main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
#define VERSION "2.0.0 - indev"

int CopyArgument(char **dst, char *src);
int ProcessSetArgument(ID3V2 *id3v2, const unsigned int ID, char *argument, unsigned char encoding);
int ProcessGetArgument(ID3V2 *id3v2, const unsigned int ID, const char *name);
int StoreArtwork(ID3V2 *id3v2, char *storepath);
int ShowFramelist(ID3V2 *id3v2);
int DumpFrame(ID3V2 *id3v2, char *frameid);
int GetEncoding(char *codename, unsigned char *encoding);
int ProcessSetArgument(ID3V2 *id3v2, unsigned int ID, const char *argument, unsigned char encoding);
int ProcessGetArgument(const ID3V2 *id3v2, const unsigned int ID, const char *name);
int StoreArtwork(ID3V2 *id3v2, const char *storepath);
int ShowFramelist(const ID3V2 *id3v2);
int DumpFrame(const ID3V2 *id3v2, const char *frameid);
int GetEncoding(const char *codename, unsigned char *encoding);
void SafeFree(void* addr);
int ValidatePath(char **path, int accessmode); // makes path absolute and checks access accessmode: W_OK|R_OK

Expand Down Expand Up @@ -336,7 +336,7 @@ int main(int argc, char *argv[])
//////////////////////////////////////////////////////////////////////////////

// Does not change anything when argument == NULL
int ProcessSetArgument(ID3V2 *id3v2, const unsigned int ID, char *argument, unsigned char encoding)
int ProcessSetArgument(ID3V2 *id3v2, unsigned int ID, const char *argument, unsigned char encoding)
{
if(argument == NULL)
return 0;
Expand Down Expand Up @@ -365,8 +365,8 @@ int ProcessSetArgument(ID3V2 *id3v2, const unsigned int ID, char *argument, unsi

case 'APIC':
{
void *picture = NULL;
unsigned int picsize;
void *picture = NULL;
size_t picsize;
error = RAWFILE_Read(argument, &picture, &picsize);
if(error)
{
Expand Down Expand Up @@ -399,7 +399,7 @@ int ProcessSetArgument(ID3V2 *id3v2, const unsigned int ID, char *argument, unsi

//----------------------------------------------------------------------------

int ProcessGetArgument(ID3V2 *id3v2, const unsigned int ID, const char *name)
int ProcessGetArgument(const ID3V2 *id3v2, unsigned int ID, const char *name)
{
int error;
size_t bufferlimit = 1024;
Expand Down Expand Up @@ -456,7 +456,7 @@ int ProcessGetArgument(ID3V2 *id3v2, const unsigned int ID, const char *name)

//----------------------------------------------------------------------------

int StoreArtwork(ID3V2 *id3v2, char *storepath)
int StoreArtwork(ID3V2 *id3v2, const char *storepath)
{
char *mimetype = NULL;
void *picture = NULL;
Expand All @@ -478,10 +478,10 @@ int StoreArtwork(ID3V2 *id3v2, char *storepath)

//----------------------------------------------------------------------------

int ShowFramelist(ID3V2 *id3v2)
int ShowFramelist(const ID3V2 *id3v2)
{
// start printing frames
ID3V2_FRAME *frame;
ID3V2_FRAME *frame;
unsigned char majorversion;

frame = id3v2->framelist;
Expand Down Expand Up @@ -581,7 +581,7 @@ int ShowFramelist(ID3V2 *id3v2)
printf("%s 0x%04X ", (frame->flags == 0x0000)?"\e[1;34m":"\e[1;31m", frame->flags);

// if frame is a text-frame, get some more infos - also for APIC
unsigned char *data = (unsigned char*)frame->data;
unsigned char *data;
data = (unsigned char*)frame->data;
if((frame->ID >> 24) == 'T')
{
Expand All @@ -603,7 +603,7 @@ int ShowFramelist(ID3V2 *id3v2)
if(encoding == ID3V2TEXTENCODING_UTF16_BOM)
{
unsigned short *utf16data;
unsigned short byteorder;
unsigned short byteorder;
utf16data = (unsigned short*)&data[1];
byteorder = utf16data[0];
if(byteorder == UTF16BOM_BE)
Expand Down Expand Up @@ -662,7 +662,7 @@ int ShowFramelist(ID3V2 *id3v2)

//----------------------------------------------------------------------------

int DumpFrame(ID3V2 *id3v2, char *frameid)
int DumpFrame(const ID3V2 *id3v2, const char *frameid)
{
ID3V2_FRAME *frame;
unsigned int ID = be32toh(*(unsigned int*)frameid);
Expand Down Expand Up @@ -751,7 +751,7 @@ int DumpFrame(ID3V2 *id3v2, char *frameid)
//////////////////////////////////////////////////////////////////////////////

// If encoding == NULL; only check if the name is valid
int GetEncoding(char *codename, unsigned char *encoding)
int GetEncoding(const char *codename, unsigned char *encoding)
{
// Make string uniform
size_t size;
Expand Down Expand Up @@ -825,7 +825,7 @@ int CopyArgument(char **dst, char *src)
if(*dst != NULL)
return -1;

int length = strlen(src);
size_t length = strlen(src);
*dst = malloc(sizeof(char)*length+1); // +1 for the \0
if(*dst == NULL)
{
Expand Down
Loading

0 comments on commit e97ffe2

Please sign in to comment.