Skip to content

Commit

Permalink
Reworked the error reporting API. Now we use error codes instead of s…
Browse files Browse the repository at this point in the history 
…trings.

This is based on work originally done by Christoph Nelles.
  • Loading branch information
@icculus
icculus committed Mar 20, 2012
1 parent 8d7cf56 commit ba676b2
Show file tree
Hide file tree
Showing 20 changed files with 882 additions and 1,184 deletions.
7 changes: 1 addition & 6 deletions docs/TODO.txt
View file
Expand Up @@ -14,10 +14,6 @@ From http://icculus.org/pipermail/physfs/2009-March/000698.html ...
- Archives formats provided by the implementation.
- Write support for various archives. I haven't decided how to do this yet,
but I'd like to.
- Replace the existing error strings with something more flexible...right now,
you have to pick a translation at compile time, which isn't too useful. It
might be nice to have real error codes for apps instead of just error
messages for humans, too.
- Add an API to expose a file's extended attributes to the application?
- Deprecate PHYSFS_setSaneConfig(). It really should have been in the extras
directory.
Expand All @@ -26,7 +22,6 @@ From http://icculus.org/pipermail/physfs/2009-March/000698.html ...

From http://icculus.org/pipermail/physfs/2010-January/000821.html ...

- Using error codes instead of error messages
- Plugin system for the archive handlers


Expand Down Expand Up @@ -86,9 +81,9 @@ Other stuff I thought of...
- bzip2 support in zip archiver?
- rewrite 7zip archiver.
- ryanify iso9660 code.
- Examine Chris Nelles's errorcode stuff.
- Move archiver_qpak.c code to archiver_unpacked.c?
- Cache basedir/userdir results (do we do this already?)
- Reduce the BAIL and GOTO macro use. A lot of these don't add anything.

Probably other stuff. Requests and recommendations are welcome.

Expand Down
23 changes: 13 additions & 10 deletions src/archiver_dir.c
View file
Expand Up @@ -15,7 +15,7 @@

static char *cvtToDependent(const char *prepend, const char *path, char *buf)
{
BAIL_IF_MACRO(buf == NULL, ERR_OUT_OF_MEMORY, NULL);
BAIL_IF_MACRO(buf == NULL, PHYSFS_ERR_OUT_OF_MEMORY, NULL);
sprintf(buf, "%s%s", prepend ? prepend : "", path);

if (__PHYSFS_platformDirSeparator != '/')
Expand All @@ -38,20 +38,23 @@ static char *cvtToDependent(const char *prepend, const char *path, char *buf)

static void *DIR_openArchive(PHYSFS_Io *io, const char *name, int forWriting)
{
PHYSFS_Stat statbuf;
PHYSFS_Stat st;
const char dirsep = __PHYSFS_platformDirSeparator;
char *retval = NULL;
const size_t namelen = strlen(name);
const size_t seplen = 1;
int exists = 0;

assert(io == NULL); /* shouldn't create an Io for these. */
BAIL_IF_MACRO(!__PHYSFS_platformStat(name, &exists, &statbuf), NULL, NULL);
if ((!exists) || (statbuf.filetype != PHYSFS_FILETYPE_DIRECTORY))
BAIL_MACRO(ERR_NOT_AN_ARCHIVE, NULL);
BAIL_IF_MACRO(!__PHYSFS_platformStat(name, &exists, &st), ERRPASS, NULL);
/* !!! FIXME: the failed Stat() call will BAIL before we check (exists) */
/* !!! FIXME: ...should we even be checking for existance here...? */
BAIL_IF_MACRO(!exists, PHYSFS_ERR_NO_SUCH_PATH, NULL);
if (st.filetype != PHYSFS_FILETYPE_DIRECTORY)
BAIL_MACRO(PHYSFS_ERR_UNSUPPORTED, NULL);

retval = allocator.Malloc(namelen + seplen + 1);
BAIL_IF_MACRO(retval == NULL, ERR_OUT_OF_MEMORY, NULL);
BAIL_IF_MACRO(retval == NULL, PHYSFS_ERR_OUT_OF_MEMORY, NULL);

strcpy(retval, name);

Expand Down Expand Up @@ -90,7 +93,7 @@ static PHYSFS_Io *doOpen(dvoid *opaque, const char *name,
int existtmp = 0;

CVT_TO_DEPENDENT(f, opaque, name);
BAIL_IF_MACRO(f == NULL, NULL, NULL);
BAIL_IF_MACRO(!f, ERRPASS, NULL);

if (fileExists == NULL)
fileExists = &existtmp;
Expand Down Expand Up @@ -136,7 +139,7 @@ static int DIR_remove(dvoid *opaque, const char *name)
char *f;

CVT_TO_DEPENDENT(f, opaque, name);
BAIL_IF_MACRO(f == NULL, NULL, 0);
BAIL_IF_MACRO(!f, ERRPASS, 0);
retval = __PHYSFS_platformDelete(f);
__PHYSFS_smallFree(f);
return retval;
Expand All @@ -149,7 +152,7 @@ static int DIR_mkdir(dvoid *opaque, const char *name)
char *f;

CVT_TO_DEPENDENT(f, opaque, name);
BAIL_IF_MACRO(f == NULL, NULL, 0);
BAIL_IF_MACRO(!f, ERRPASS, 0);
retval = __PHYSFS_platformMkDir(f);
__PHYSFS_smallFree(f);
return retval;
Expand All @@ -169,7 +172,7 @@ static int DIR_stat(dvoid *opaque, const char *name, int *exists,
char *d;

CVT_TO_DEPENDENT(d, opaque, name);
BAIL_IF_MACRO(d == NULL, NULL, 0);
BAIL_IF_MACRO(!d, ERRPASS, 0);
retval = __PHYSFS_platformStat(d, exists, stat);
__PHYSFS_smallFree(d);
return retval;
Expand Down
16 changes: 8 additions & 8 deletions src/archiver_grp.c
View file
Expand Up @@ -37,14 +37,14 @@ static UNPKentry *grpLoadEntries(PHYSFS_Io *io, PHYSFS_uint32 fileCount)
char *ptr = NULL;

entries = (UNPKentry *) allocator.Malloc(sizeof (UNPKentry) * fileCount);
BAIL_IF_MACRO(entries == NULL, ERR_OUT_OF_MEMORY, NULL);
BAIL_IF_MACRO(entries == NULL, PHYSFS_ERR_OUT_OF_MEMORY, NULL);

location += (16 * fileCount);

for (entry = entries; fileCount > 0; fileCount--, entry++)
{
GOTO_IF_MACRO(!__PHYSFS_readAll(io, &entry->name, 12), NULL, failed);
GOTO_IF_MACRO(!__PHYSFS_readAll(io, &entry->size, 4), NULL, failed);
GOTO_IF_MACRO(!__PHYSFS_readAll(io, &entry->name, 12), ERRPASS, failed);
GOTO_IF_MACRO(!__PHYSFS_readAll(io, &entry->size, 4), ERRPASS, failed);
entry->name[12] = '\0'; /* name isn't null-terminated in file. */
if ((ptr = strchr(entry->name, ' ')) != NULL)
*ptr = '\0'; /* trim extra spaces. */
Expand All @@ -70,17 +70,17 @@ static void *GRP_openArchive(PHYSFS_Io *io, const char *name, int forWriting)

assert(io != NULL); /* shouldn't ever happen. */

BAIL_IF_MACRO(forWriting, ERR_ARC_IS_READ_ONLY, 0);
BAIL_IF_MACRO(forWriting, PHYSFS_ERR_READ_ONLY, 0);

BAIL_IF_MACRO(!__PHYSFS_readAll(io, buf, sizeof (buf)), NULL, NULL);
BAIL_IF_MACRO(!__PHYSFS_readAll(io, buf, sizeof (buf)), ERRPASS, NULL);
if (memcmp(buf, "KenSilverman", sizeof (buf)) != 0)
BAIL_MACRO(ERR_NOT_AN_ARCHIVE, NULL);
BAIL_MACRO(PHYSFS_ERR_UNSUPPORTED, NULL);

BAIL_IF_MACRO(!__PHYSFS_readAll(io, &count, sizeof (count)), NULL, NULL);
BAIL_IF_MACRO(!__PHYSFS_readAll(io, &count, sizeof(count)), ERRPASS, NULL);
count = PHYSFS_swapULE32(count);

entries = grpLoadEntries(io, count);
BAIL_IF_MACRO(entries == NULL, NULL, NULL);
BAIL_IF_MACRO(!entries, ERRPASS, NULL);
return UNPK_openArchive(io, entries, count);
} /* GRP_openArchive */

Expand Down
16 changes: 8 additions & 8 deletions src/archiver_hog.c
View file
Expand Up @@ -48,21 +48,21 @@ static UNPKentry *hogLoadEntries(PHYSFS_Io *io, PHYSFS_uint32 *_entCount)
{
entCount++;
ptr = allocator.Realloc(ptr, sizeof (UNPKentry) * entCount);
GOTO_IF_MACRO(ptr == NULL, ERR_OUT_OF_MEMORY, failed);
GOTO_IF_MACRO(ptr == NULL, PHYSFS_ERR_OUT_OF_MEMORY, failed);
entries = (UNPKentry *) ptr;
entry = &entries[entCount-1];

GOTO_IF_MACRO(!__PHYSFS_readAll(io, &entry->name, 13), NULL, failed);
GOTO_IF_MACRO(!__PHYSFS_readAll(io, &entry->name, 13), ERRPASS, failed);
pos += 13;
GOTO_IF_MACRO(!__PHYSFS_readAll(io, &size, 4), NULL, failed);
GOTO_IF_MACRO(!__PHYSFS_readAll(io, &size, 4), ERRPASS, failed);
pos += 4;

entry->size = PHYSFS_swapULE32(size);
entry->startPos = pos;
pos += size;

/* skip over entry */
GOTO_IF_MACRO(!io->seek(io, pos), NULL, failed);
GOTO_IF_MACRO(!io->seek(io, pos), ERRPASS, failed);
} /* while */

*_entCount = entCount;
Expand All @@ -81,12 +81,12 @@ static void *HOG_openArchive(PHYSFS_Io *io, const char *name, int forWriting)
UNPKentry *entries = NULL;

assert(io != NULL); /* shouldn't ever happen. */
BAIL_IF_MACRO(forWriting, ERR_ARC_IS_READ_ONLY, 0);
BAIL_IF_MACRO(!__PHYSFS_readAll(io, buf, 3), NULL, 0);
BAIL_IF_MACRO(memcmp(buf, "DHF", 3) != 0, ERR_NOT_AN_ARCHIVE, NULL);
BAIL_IF_MACRO(forWriting, PHYSFS_ERR_READ_ONLY, NULL);
BAIL_IF_MACRO(!__PHYSFS_readAll(io, buf, 3), ERRPASS, NULL);
BAIL_IF_MACRO(memcmp(buf, "DHF", 3) != 0, PHYSFS_ERR_UNSUPPORTED, NULL);

entries = hogLoadEntries(io, &count);
BAIL_IF_MACRO(entries == NULL, NULL, NULL);
BAIL_IF_MACRO(!entries, ERRPASS, NULL);
return UNPK_openArchive(io, entries, count);
} /* HOG_openArchive */

Expand Down

0 comments on commit ba676b2

Please sign in to comment.