From 6dbf0dced96ab6144f17e9931fd258c0d7a594ad Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Mon, 23 Sep 2002 20:32:26 +0000 Subject: [PATCH] Fixed minor bugs when trying to open a file that doesn't actually contain an archive. Spotted by valgrind. --- nufxlib-0/Archive.c | 17 ++++++-- nufxlib-0/ArchiveIO.c | 29 ++++++++------ nufxlib-0/MiscStuff.h | 5 ++- nufxlib-0/configure | 90 +++++++++++++++++++++---------------------- nulib2/ArcUtils.c | 15 +++++++- nulib2/MiscStuff.h | 5 ++- nulib2/configure | 54 +++++++++++++------------- 7 files changed, 121 insertions(+), 94 deletions(-) diff --git a/nufxlib-0/Archive.c b/nufxlib-0/Archive.c index ccc23db..c0c6f03 100644 --- a/nufxlib-0/Archive.c +++ b/nufxlib-0/Archive.c @@ -40,6 +40,9 @@ static const uchar kNuSHKSEAID[] = #define kNuSEALength2 12001 /* length of archive (4B?) */ +static void Nu_CloseAndFree(NuArchive* pArchive); + + /* * =========================================================================== * Archive and MasterHeader utility functions @@ -703,6 +706,7 @@ Nu_OpenRO(const char* archivePathname, NuArchive** ppArchive) pArchive->openMode = kNuOpenRO; pArchive->archiveFp = fp; + fp = nil; pArchive->archivePathname = strdup(archivePathname); err = Nu_ReadMasterHeader(pArchive); @@ -710,8 +714,10 @@ Nu_OpenRO(const char* archivePathname, NuArchive** ppArchive) bail: if (err != kNuErrNone) { - if (pArchive != nil) - (void) Nu_NuArchiveFree(pArchive); + if (pArchive != nil) { + (void) Nu_CloseAndFree(pArchive); + *ppArchive = nil; + } if (fp != nil) fclose(fp); } @@ -908,8 +914,8 @@ Nu_OpenRW(const char* archivePathname, const char* tmpPathname, ulong flags, pArchive->openMode = kNuOpenRW; pArchive->newlyCreated = newlyCreated; - pArchive->archiveFp = fp; pArchive->archivePathname = strdup(archivePathname); + pArchive->archiveFp = fp; fp = nil; pArchive->tmpFp = tmpFp; tmpFp = nil; @@ -926,7 +932,7 @@ Nu_OpenRW(const char* archivePathname, const char* tmpPathname, ulong flags, bail: if (err != kNuErrNone) { if (pArchive != nil) { - (void) Nu_NuArchiveFree(pArchive); + (void) Nu_CloseAndFree(pArchive); *ppArchive = nil; } if (fp != nil) @@ -1063,6 +1069,9 @@ Nu_Close(NuArchive* pArchive) DBUG(("--- Close NuFlush status was 0x%4lx\n", flushStatus)); } + if (err != kNuErrNone) { + DBUG(("--- Nu_Close returning error %d\n", err)); + } return err; } diff --git a/nufxlib-0/ArchiveIO.c b/nufxlib-0/ArchiveIO.c index 2484a29..2435cc6 100644 --- a/nufxlib-0/ArchiveIO.c +++ b/nufxlib-0/ArchiveIO.c @@ -12,6 +12,11 @@ #include "NufxLibPriv.h" +/* this makes valgrind and purify happy, at some tiny cost in speed */ +#define CLEAN_INIT =0 +/*#define CLEAN_INIT */ + + /* * =========================================================================== * Read and write @@ -39,7 +44,7 @@ Nu_ReadOneC(NuArchive* pArchive, FILE* fp, ushort* pCrc) uchar Nu_ReadOne(NuArchive* pArchive, FILE* fp) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; return Nu_ReadOneC(pArchive, fp, &dummyCrc); } @@ -59,7 +64,7 @@ Nu_WriteOneC(NuArchive* pArchive, FILE* fp, uchar val, ushort* pCrc) void Nu_WriteOne(NuArchive* pArchive, FILE* fp, uchar val) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; Nu_WriteOneC(pArchive, fp, val, &dummyCrc); } @@ -87,7 +92,7 @@ Nu_ReadTwoC(NuArchive* pArchive, FILE* fp, ushort* pCrc) ushort Nu_ReadTwo(NuArchive* pArchive, FILE* fp) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; return Nu_ReadTwoC(pArchive, fp, &dummyCrc); } @@ -116,7 +121,7 @@ Nu_WriteTwoC(NuArchive* pArchive, FILE* fp, ushort val, ushort* pCrc) void Nu_WriteTwo(NuArchive* pArchive, FILE* fp, ushort val) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; Nu_WriteTwoC(pArchive, fp, val, &dummyCrc); } @@ -148,7 +153,7 @@ Nu_ReadFourC(NuArchive* pArchive, FILE* fp, ushort* pCrc) ulong Nu_ReadFour(NuArchive* pArchive, FILE* fp) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; return Nu_ReadFourC(pArchive, fp, &dummyCrc); } @@ -183,7 +188,7 @@ Nu_WriteFourC(NuArchive* pArchive, FILE* fp, ulong val, ushort* pCrc) void Nu_WriteFour(NuArchive* pArchive, FILE* fp, ulong val) { - ushort dummyCrc /*=0*/; + ushort dummyCrc CLEAN_INIT; Nu_WriteFourC(pArchive, fp, val, &dummyCrc); } @@ -236,7 +241,7 @@ Nu_ReadDateTimeC(NuArchive* pArchive, FILE* fp, ushort* pCrc) NuDateTime Nu_ReadDateTime(NuArchive* pArchive, FILE* fp, ushort* pCrc) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; return Nu_ReadDateTimeC(pArchive, fp, &dummyCrc); } @@ -283,7 +288,7 @@ Nu_WriteDateTimeC(NuArchive* pArchive, FILE* fp, NuDateTime dateTime, void Nu_WriteDateTime(NuArchive* pArchive, FILE* fp, NuDateTime dateTime) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; Nu_WriteDateTimeC(pArchive, fp, dateTime, &dummyCrc); } @@ -314,7 +319,7 @@ Nu_ReadBytesC(NuArchive* pArchive, FILE* fp, void* vbuffer, long count, void Nu_ReadBytes(NuArchive* pArchive, FILE* fp, void* vbuffer, long count) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; Nu_ReadBytesC(pArchive, fp, vbuffer, count, &dummyCrc); } @@ -345,7 +350,7 @@ Nu_WriteBytesC(NuArchive* pArchive, FILE* fp, const void* vbuffer, long count, void Nu_WriteBytes(NuArchive* pArchive, FILE* fp, const void* vbuffer, long count) { - ushort dummyCrc /*= 0*/; + ushort dummyCrc CLEAN_INIT; Nu_WriteBytesC(pArchive, fp, vbuffer, count, &dummyCrc); } @@ -381,9 +386,9 @@ Nu_SeekArchive(NuArchive* pArchive, FILE* fp, long offset, int ptrname) { if (Nu_IsStreaming(pArchive)) { Assert(ptrname == SEEK_CUR); - Assert(offset > 0); + Assert(offset >= 0); - /* might be faster to fread a chunk at a time */ + /* OPT: might be faster to fread a chunk at a time */ while (offset--) (void) getc(fp); diff --git a/nufxlib-0/MiscStuff.h b/nufxlib-0/MiscStuff.h index 5455fad..cabbf35 100644 --- a/nufxlib-0/MiscStuff.h +++ b/nufxlib-0/MiscStuff.h @@ -9,6 +9,8 @@ #ifndef __MiscStuff__ #define __MiscStuff__ +#define VALGRIND /* assume we're using it */ + #include "SysDefs.h" /* @@ -93,7 +95,7 @@ typedef uchar Boolean; #else /* when debugging, fill Malloc blocks with junk, unless we're using Purify */ - #if !defined(PURIFY) + #if !defined(PURIFY) && !defined(VALGRIND) #define DebugFill(addr, len) memset(addr, 0xa3, len) #else #define DebugFill(addr, len) ((void)0) @@ -102,7 +104,6 @@ typedef uchar Boolean; #define DebugAbort() abort() #endif -#define kInvalidFill (0xa3) #define kInvalidPtr ((void*)0xa3a3a3a3) #endif /*__MiscStuff__*/ diff --git a/nufxlib-0/configure b/nufxlib-0/configure index 08c2fb4..67f2902 100755 --- a/nufxlib-0/configure +++ b/nufxlib-0/configure @@ -997,7 +997,7 @@ fi echo "$ac_t""$CPP" 1>&6 for ac_hdr in fcntl.h malloc.h stdlib.h sys/stat.h sys/time.h sys/types.h \ - sys/utime.h unistd.h utime.h + sys/utime.h unistd.h utime.h do ac_safe=`echo "$ac_hdr" | sed 'y%./+-%__p_%'` echo $ac_n "checking for $ac_hdr""... $ac_c" 1>&6 @@ -1524,7 +1524,7 @@ fi for ac_func in fdopen ftruncate memmove mkdir mkstemp mktime timelocal \ - localtime_r snprintf strcasecmp strncasecmp strtoul strerror vsnprintf + localtime_r snprintf strcasecmp strncasecmp strtoul strerror vsnprintf do echo $ac_n "checking for $ac_func""... $ac_c" 1>&6 echo "configure:1531: checking for $ac_func" >&5 @@ -1586,7 +1586,7 @@ if eval "test \"`echo '$''{'nufxlib_cv_snprintf_in_header'+set}'`\" = set"; then echo $ac_n "(cached) $ac_c" 1>&6 else - cat > conftest.$ac_ext < conftest.$ac_ext < @@ -1605,7 +1605,7 @@ rm -f conftest* fi if test $nufxlib_cv_snprintf_in_header = "yes"; then - cat >> confdefs.h <<\EOF + cat >> confdefs.h <<\EOF #define SNPRINTF_DECLARED 1 EOF @@ -1618,7 +1618,7 @@ if eval "test \"`echo '$''{'nufxlib_cv_vsnprintf_in_header'+set}'`\" = set"; the echo $ac_n "(cached) $ac_c" 1>&6 else - cat > conftest.$ac_ext < conftest.$ac_ext < @@ -1637,7 +1637,7 @@ rm -f conftest* fi if test $nufxlib_cv_vsnprintf_in_header = "yes"; then - cat >> confdefs.h <<\EOF + cat >> confdefs.h <<\EOF #define VSNPRINTF_DECLARED 1 EOF @@ -1646,19 +1646,19 @@ echo "$ac_t""$nufxlib_cv_vsnprintf_in_header" 1>&6 SHARE_FLAGS='-shared' if test "$host_cpu" = "powerpc" -a "$host_os" = "beos"; then - CC=cc - GCC= - CFLAGS='-proc 603 -opt full' - SHARE_FLAGS='-shared -nostdlib' - echo "forcing CC to \"$CC\" and CFLAGS to \"$CFLAGS\"" + CC=cc + GCC= + CFLAGS='-proc 603 -opt full' + SHARE_FLAGS='-shared -nostdlib' + echo "forcing CC to \"$CC\" and CFLAGS to \"$CFLAGS\"" elif test "$host_os" = "beos"; then - SHARE_FLAGS='-nostartfiles -Xlinker -soname="$@"' + SHARE_FLAGS='-nostartfiles -Xlinker -soname="$@"' fi if test -z "$GCC"; then - BUILD_FLAGS='$(OPT)' + BUILD_FLAGS='$(OPT)' else - BUILD_FLAGS='$(OPT) $(GCC_FLAGS)' + BUILD_FLAGS='$(OPT) $(GCC_FLAGS)' fi @@ -1666,24 +1666,24 @@ fi if test "$host_os" = "beos"; then - if test "$prefix" = "NONE" -a \ - "$includedir" = '${prefix}/include' -a \ - "$libdir" = '${exec_prefix}/lib' -a \ - "$bindir" = '${exec_prefix}/bin' -a \ - "$mandir" = '${prefix}/man' - then - echo replacing install locations with BeOS values - prefix=/boot - includedir='${prefix}/develop/headers' - libdir='${exec_prefix}/home/config/lib' - bindir='${exec_prefix}/home/config/bin' - mandir='/tmp' - - - - - - fi + if test "$prefix" = "NONE" -a \ + "$includedir" = '${prefix}/include' -a \ + "$libdir" = '${exec_prefix}/lib' -a \ + "$bindir" = '${exec_prefix}/bin' -a \ + "$mandir" = '${prefix}/man' + then + echo replacing install locations with BeOS values + prefix=/boot + includedir='${prefix}/develop/headers' + libdir='${exec_prefix}/home/config/lib' + bindir='${exec_prefix}/home/config/bin' + mandir='/tmp' + + + + + + fi fi echo $ac_n "checking if sprintf returns int""... $ac_c" 1>&6 @@ -1692,22 +1692,22 @@ if eval "test \"`echo '$''{'nufxlib_cv_sprintf_returns_int'+set}'`\" = set"; the echo $ac_n "(cached) $ac_c" 1>&6 else - if test "$cross_compiling" = yes; then + if test "$cross_compiling" = yes; then nufxlib_cv_sprintf_returns_int=no else cat > conftest.$ac_ext < - int main(void) - { - int count; - char buf[8]; - count = sprintf(buf, "123"); /* should return three */ - exit(count != 3); - } - + #include + int main(void) + { + int count; + char buf[8]; + count = sprintf(buf, "123"); /* should return three */ + exit(count != 3); + } + EOF if { (eval echo configure:1713: \"$ac_link\") 1>&5; (eval $ac_link) 2>&5; } && test -s conftest${ac_exeext} && (./conftest; exit) 2>/dev/null then @@ -1725,7 +1725,7 @@ fi fi if test $nufxlib_cv_sprintf_returns_int = "yes"; then - cat >> confdefs.h <<\EOF + cat >> confdefs.h <<\EOF #define SPRINTF_RETURNS_INT 1 EOF @@ -1737,8 +1737,8 @@ DMALLOC= if test "${enable_dmalloc+set}" = set; then enableval="$enable_dmalloc" \ - echo "--- enabling dmalloc"; \ - DMALLOC="-L/usr/local/lib -ldmalloc"; cat >> confdefs.h <<\EOF + echo "--- enabling dmalloc"; \ + DMALLOC="-L/usr/local/lib -ldmalloc"; cat >> confdefs.h <<\EOF #define USE_DMALLOC 1 EOF diff --git a/nulib2/ArcUtils.c b/nulib2/ArcUtils.c index f21d41e..b85a78e 100644 --- a/nulib2/ArcUtils.c +++ b/nulib2/ArcUtils.c @@ -764,7 +764,7 @@ NuError OpenArchiveReadOnly(NulibState* pState) { NuError err; - NuArchive* pArchive; + NuArchive* pArchive = nil; assert(pState != nil); @@ -844,6 +844,11 @@ OpenArchiveReadOnly(NulibState* pState) BailError(err); bail: + if (err != kNuErrNone && pArchive != nil) { + /* clean up */ + (void) NuClose(pArchive); + NState_SetNuArchive(pState, nil); + } return err; } @@ -858,7 +863,7 @@ NuError OpenArchiveReadWrite(NulibState* pState) { NuError err = kNuErrNone; - NuArchive* pArchive; + NuArchive* pArchive = nil; char* tempName = nil; assert(pState != nil); @@ -930,6 +935,12 @@ OpenArchiveReadWrite(NulibState* pState) bail: Free(tempName); + if (err != kNuErrNone && pArchive != nil) { + /* clean up */ + NuAbort(pArchive); + (void) NuClose(pArchive); + NState_SetNuArchive(pState, nil); + } return err; } diff --git a/nulib2/MiscStuff.h b/nulib2/MiscStuff.h index 5455fad..cabbf35 100644 --- a/nulib2/MiscStuff.h +++ b/nulib2/MiscStuff.h @@ -9,6 +9,8 @@ #ifndef __MiscStuff__ #define __MiscStuff__ +#define VALGRIND /* assume we're using it */ + #include "SysDefs.h" /* @@ -93,7 +95,7 @@ typedef uchar Boolean; #else /* when debugging, fill Malloc blocks with junk, unless we're using Purify */ - #if !defined(PURIFY) + #if !defined(PURIFY) && !defined(VALGRIND) #define DebugFill(addr, len) memset(addr, 0xa3, len) #else #define DebugFill(addr, len) ((void)0) @@ -102,7 +104,6 @@ typedef uchar Boolean; #define DebugAbort() abort() #endif -#define kInvalidFill (0xa3) #define kInvalidPtr ((void*)0xa3a3a3a3) #endif /*__MiscStuff__*/ diff --git a/nulib2/configure b/nulib2/configure index 9959b76..95b3e1f 100755 --- a/nulib2/configure +++ b/nulib2/configure @@ -1169,7 +1169,7 @@ EOF fi for ac_hdr in fcntl.h limits.h malloc.h stdlib.h strings.h sys/stat.h \ - sys/time.h sys/types.h unistd.h + sys/time.h sys/types.h unistd.h do ac_safe=`echo "$ac_hdr" | sed 'y%./+-%__p_%'` echo $ac_n "checking for $ac_hdr""... $ac_c" 1>&6 @@ -1653,38 +1653,38 @@ done if test "$host_os" = "beos"; then - if test "$prefix" = "NONE" -a \ - "$includedir" = '${prefix}/include' -a \ - "$libdir" = '${exec_prefix}/lib' -a \ - "$bindir" = '${exec_prefix}/bin' -a \ - "$mandir" = '${prefix}/man' - then - echo replacing install locations with BeOS values - prefix=/boot - includedir='${prefix}/develop/headers' - libdir='${exec_prefix}/home/config/lib' - bindir='${exec_prefix}/home/config/bin' - mandir='/tmp' - - - - - - fi + if test "$prefix" = "NONE" -a \ + "$includedir" = '${prefix}/include' -a \ + "$libdir" = '${exec_prefix}/lib' -a \ + "$bindir" = '${exec_prefix}/bin' -a \ + "$mandir" = '${prefix}/man' + then + echo replacing install locations with BeOS values + prefix=/boot + includedir='${prefix}/develop/headers' + libdir='${exec_prefix}/home/config/lib' + bindir='${exec_prefix}/home/config/bin' + mandir='/tmp' + + + + + + fi fi if test "$host_cpu" = "powerpc" -a "$host_os" = "beos"; then - CC=cc - GCC= - CFLAGS='-proc 603 -opt full' - echo "forcing CC to \"$CC\" and CFLAGS to \"$CFLAGS\"" + CC=cc + GCC= + CFLAGS='-proc 603 -opt full' + echo "forcing CC to \"$CC\" and CFLAGS to \"$CFLAGS\"" fi if test -z "$GCC"; then - BUILD_FLAGS='$(OPT)' + BUILD_FLAGS='$(OPT)' else - BUILD_FLAGS='$(OPT) $(GCC_FLAGS)' + BUILD_FLAGS='$(OPT) $(GCC_FLAGS)' fi @@ -1694,8 +1694,8 @@ DMALLOC= if test "${enable_dmalloc+set}" = set; then enableval="$enable_dmalloc" \ - echo "--- enabling dmalloc"; - DMALLOC="-L/usr/local/lib -ldmalloc"; cat >> confdefs.h <<\EOF + echo "--- enabling dmalloc"; + DMALLOC="-L/usr/local/lib -ldmalloc"; cat >> confdefs.h <<\EOF #define USE_DMALLOC 1 EOF