From b32a9f33f59a743589372937344d025551d667f6 Mon Sep 17 00:00:00 2001 From: "dougt%netscape.com" Date: Wed, 25 Aug 1999 21:42:16 +0000 Subject: [PATCH] Code Review Comments. Look for "//dougt:" as well as looking at the diffs I have fixed some problems, and left other merely commented. I did not make it completely through files not touched in this directory, or the *Win.c files. I am not certain that this compiles still. --- xpinstall/wizard/mac/src/ComponentsWin.c | 13 ++- xpinstall/wizard/mac/src/Deflation.c | 4 +- xpinstall/wizard/mac/src/EvtHandlers.c | 17 ++-- xpinstall/wizard/mac/src/GreyButton.cp | 6 +- xpinstall/wizard/mac/src/InstAction.c | 5 +- xpinstall/wizard/mac/src/LicenseWin.c | 29 +++--- xpinstall/wizard/mac/src/MacInstallWizard.c | 22 ++--- xpinstall/wizard/mac/src/Parser.c | 99 +++++++++++++-------- xpinstall/wizard/mac/src/SetupTypeWin.c | 13 +-- xpinstall/wizard/mac/src/TerminalWin.c | 12 +-- xpinstall/wizard/mac/src/WelcomeWin.c | 26 +++--- xpinstall/wizard/mac/src/XPInstallGlue.c | 26 +++--- 12 files changed, 150 insertions(+), 122 deletions(-) diff --git a/xpinstall/wizard/mac/src/ComponentsWin.c b/xpinstall/wizard/mac/src/ComponentsWin.c index 0739dc61baa1..69121dadbf07 100644 --- a/xpinstall/wizard/mac/src/ComponentsWin.c +++ b/xpinstall/wizard/mac/src/ComponentsWin.c @@ -20,10 +20,7 @@ * Samir Gehani */ - -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" /*-----------------------------------------------------------* @@ -54,16 +51,16 @@ ShowComponentsWin(void) // get controls listBoxRect = Get1Resource('RECT', rCompListBox); - reserr = ResError(); + reserr = ResError(); //dougt: this does not do what you think if (reserr == noErr) { - HLockHi((Handle)listBoxRect); + HLockHi((Handle)listBoxRect); //dougt: no hi. gControls->cw->compListBox = (Rect) **((Rect **)listBoxRect); HUnlock((Handle)listBoxRect); + ErrorHandler(); } else { - ErrorHandler(); return; } gControls->cw->compDescBox = GetNewControl(rCompDescBox, gWPtr); @@ -71,7 +68,7 @@ ShowComponentsWin(void) gControls->cw->compListBox.right -= kScrollBarWidth; SetRect(&dataBounds, 0, 0, 1, gControls->cfg->numComps); SetPt( &cSize, 0, 0); - gControls->cw->compList = lnew(&gControls->cw->compListBox, &dataBounds, + gControls->cw->compList = lnew(&gControls->cw->compListBox, &dataBounds, //dougt: what is this call!! it should be LNew()?? &cSize, 0, gWPtr, TRUE, FALSE, FALSE, TRUE); // populate controls diff --git a/xpinstall/wizard/mac/src/Deflation.c b/xpinstall/wizard/mac/src/Deflation.c index cf68044d2161..2fb8f71b874a 100644 --- a/xpinstall/wizard/mac/src/Deflation.c +++ b/xpinstall/wizard/mac/src/Deflation.c @@ -21,9 +21,7 @@ */ -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" #define STANDALONE 1 #define XP_MAC 1 diff --git a/xpinstall/wizard/mac/src/EvtHandlers.c b/xpinstall/wizard/mac/src/EvtHandlers.c index 7286612000f6..3b9b6caf167f 100644 --- a/xpinstall/wizard/mac/src/EvtHandlers.c +++ b/xpinstall/wizard/mac/src/EvtHandlers.c @@ -20,10 +20,7 @@ * Samir Gehani */ - -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" /*--------------------------------------------------------------* @@ -82,7 +79,7 @@ void HandleMouseDown(EventRecord* evt) SelectWindow(wCurrPtr); } else - React2InContent(evt, wCurrPtr); + React2InContent(evt, wCurrPtr); //dougt: what does this do? break; case inDrag: @@ -103,8 +100,8 @@ void HandleKeyDown(EventRecord* evt) keyPressed = evt->message & charCodeMask; if ( (keyPressed == 'z') || (keyPressed == 'Z')) - gDone = true; // backdoor exit - if (keyPressed == '\r') + gDone = true; // backdoor exit //dougt: Get rid of this. (or make it debug only) + if (keyPressed == '\r') //dougt: what about tab, esc, arrows, doublebyte? { switch(gCurrWin) { @@ -150,7 +147,7 @@ void HandleUpdateEvt(EventRecord* evt) SetPort( gWPtr ); cntlPartCode = FindWindow( evt->where, &wCurrPtr ); - + //dougt: check for null BeginUpdate( gWPtr ); DrawControls( gWPtr ); ShowLogo(); @@ -199,7 +196,7 @@ void HandleActivateEvt(EventRecord* evt) void HandleOSEvt(EventRecord* evt) { - switch ( (evt->message >> 24) & 0x000000FF) + switch ( (evt->message >> 24) & 0x000000FF) //dougt: Okay, what is this? { case suspendResumeMessage: if ((evt->message & resumeFlag) == 1) @@ -276,7 +273,7 @@ void React2InContent(EventRecord* evt, WindowPtr wCurrPtr) break; default: - gDone = true; + gDone = true; //dougt: are you sure you want to do this? break; } } diff --git a/xpinstall/wizard/mac/src/GreyButton.cp b/xpinstall/wizard/mac/src/GreyButton.cp index 2e9cd27e864e..fee9fb8e9c1a 100644 --- a/xpinstall/wizard/mac/src/GreyButton.cp +++ b/xpinstall/wizard/mac/src/GreyButton.cp @@ -19,17 +19,15 @@ * Contributors: * Michael C. Amorose */ - +//dougt: you need to send michael@michael-amorose.com mail about using this. /*---------------------------------------------------------------------* * Routines for drawing standard AGA pushbutton. * Loosely based on code by Zig Zichterman. *---------------------------------------------------------------------*/ +#include "MacInstallWizard.h" -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif pascal void FrameGreyButton( Rect *buttonFrame ) { diff --git a/xpinstall/wizard/mac/src/InstAction.c b/xpinstall/wizard/mac/src/InstAction.c index 0ca4f171e7cc..b390c91ac2cc 100644 --- a/xpinstall/wizard/mac/src/InstAction.c +++ b/xpinstall/wizard/mac/src/InstAction.c @@ -20,10 +20,7 @@ * Samir Gehani */ - -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" /*-----------------------------------------------------------* diff --git a/xpinstall/wizard/mac/src/LicenseWin.c b/xpinstall/wizard/mac/src/LicenseWin.c index f0f286a5c3c6..d5935215b1b4 100644 --- a/xpinstall/wizard/mac/src/LicenseWin.c +++ b/xpinstall/wizard/mac/src/LicenseWin.c @@ -21,9 +21,7 @@ */ -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" /*-----------------------------------------------------------* @@ -42,7 +40,8 @@ ShowLicenseWin(void) GetPort(&oldPort); SetPort(gWPtr); - + +//dougt: nitpick, your use of the define LICENSE is vage. I had no initial idea of what it was. How about kLicenseID? gCurrWin = LICENSE; /* gControls->lw = (LicWin *) NewPtrClear(sizeof(LicWin)); */ @@ -51,9 +50,11 @@ ShowLicenseWin(void) gControls->lw->scrollBar = GetNewControl( rLicScrollBar, gWPtr); gControls->lw->licBox = GetNewControl( rLicBox, gWPtr); - +//dougt: what happens if these fail? don't you want to bail instead of just checking on the next line? if(gControls->lw->scrollBar && gControls->lw->licBox) { + //dougt: you don't need to lock hi here. + HLockHi( (Handle) gControls->lw->scrollBar); sbRect = (*(gControls->lw->licBox))->contrlRect; @@ -94,6 +95,7 @@ InitLicTxt(void) ERR_CHECK(GetCWD(&dirID, &vRefNum)); /* open and read license file */ +//dougt: no need to lock hi., HLockHi(gControls->cfg->licFileName); if(**gControls->cfg->licFileName != nil) { @@ -101,6 +103,7 @@ InitLicTxt(void) cLicFName = CToPascal(*gControls->cfg->licFileName); ERR_CHECK(FSMakeFSSpec(vRefNum, dirID, cLicFName, &licFile)); +//dougt: on any dispose, check for null! DisposePtr((char*)cLicFName); } else /* assume default license filename from str rsrc */ @@ -117,7 +120,7 @@ InitLicTxt(void) if (dataSize > 0) { if (!(text = NewHandle(dataSize))) - ErrorHandler(); + ErrorHandler(); //dougt: since errorhandler() return, you will crash on the next line ERR_CHECK(FSRead(dataRef, &dataSize, *text)); } else @@ -198,7 +201,7 @@ ShowTxt(void) } break; default: - ErrorHandler(); + ErrorHandler(); //dougt: i don;t think so tim, break; } } @@ -212,17 +215,19 @@ ShowLogo(void) Handle logoRectH; /* initialize Netscape logo */ - logoPicH = GetPicture(rNSLogo); + logoPicH = GetPicture(rNSLogo); //dougt: isn;t this something that should be pulled from an ini file? Also, what about + // better error handling? /* draw Netscape logo */ if (logoPicH != nil) { logoRectH = Get1Resource('RECT', rNSLogoBox); - HLockHi(logoRectH); + //dougt: check failure + HLockHi(logoRectH);//dougt: no lock hi derefd = (Rect) **((Rect**)logoRectH); SetRect(&logoRect, derefd.left, derefd.top, derefd.bottom, derefd.right); HUnlock(logoRectH); - reserr = ResError(); + reserr = ResError(); //dougt: checking this does not gaurentee you the correct ResError(). if (reserr == noErr) { DrawPicture(logoPicH, &logoRect); @@ -435,7 +440,7 @@ ShowNavButtons(unsigned char* backTitle, unsigned char* nextTitle) gControls->backB = GetNewControl( rBackBtn, gWPtr); gControls->nextB = GetNewControl( rNextBtn, gWPtr); - +//dougt: check for failure... if( gControls->backB != NULL) { SetControlTitle( gControls->backB, backTitle); @@ -446,7 +451,7 @@ ShowNavButtons(unsigned char* backTitle, unsigned char* nextTitle) { SetControlTitle( gControls->nextB, nextTitle); ShowControl( gControls->nextB); - +//dougt: no hi. HLockHi( (Handle) gControls->nextB); bounds = (*(gControls->nextB))->contrlRect; diff --git a/xpinstall/wizard/mac/src/MacInstallWizard.c b/xpinstall/wizard/mac/src/MacInstallWizard.c index 8c09eb7dca13..5eb2e539c417 100644 --- a/xpinstall/wizard/mac/src/MacInstallWizard.c +++ b/xpinstall/wizard/mac/src/MacInstallWizard.c @@ -20,15 +20,12 @@ * Samir Gehani */ - -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif - +#include "MacInstallWizard.h" /*-----------------------------------------------------------* * globals *-----------------------------------------------------------*/ +//dougt: maybe you should init these here to null Boolean gDone; WindowPtr gWPtr; short gCurrWin; @@ -75,6 +72,7 @@ void Init(void) ErrorHandler(); return; } +//dougt: will sd put up some ui while the main thread blocks? /* block/busy wait till download finishes */ while (1) @@ -91,7 +89,7 @@ void Init(void) #endif /* CFG_IS_REMOTE == 1 */ gWPtr = GetNewCWindow(rRootWin, NULL, (WindowPtr) -1); - GetIndString( winTitle, rStringList, sNSInstTitle); + GetIndString( winTitle, rStringList, sNSInstTitle); //pstrcpy(winTitle, "\pNetscape Installer Dude"); SetWTitle( gWPtr, winTitle ); MakeMenus(); @@ -135,11 +133,11 @@ InitOptObject(void) OSErr err=noErr; gControls->opt = (Options*)NewPtrClear(sizeof(Options)); - +//dougt: what happens when allocation fails! /* SetupTypeWin options */ gControls->opt->instChoice = 1; gControls->opt->folder = (unsigned char *)NewPtrClear(64*sizeof(unsigned char)); - +//dougt: what happens when allocation fails! ERR_CHECK(GetCWD(&gControls->opt->dirID, &gControls->opt->vRefNum)); ERR_CHECK(FSMakeFSSpec(gControls->opt->vRefNum, gControls->opt->dirID, NULL, &tmp)); @@ -159,6 +157,7 @@ InitControlsObject(void) gControls->stw = (SetupTypeWin *) NewPtrClear(sizeof(SetupTypeWin)); gControls->cw = (CompWin *) NewPtrClear(sizeof(CompWin)); gControls->tw = (TermWin*) NewPtrClear(sizeof(TermWin)); +//dougt: what happens when allocation fails! } void InitManagers(void) @@ -179,13 +178,14 @@ void InitManagers(void) void MakeMenus(void) { - Handle mbarHdl; +//dougt: the use of ErrorHandler is wrong here. Since it will not 'exit to shell', execution will continue which is not desired. + Handle mbarHdl; MenuHandle menuHdl; OSErr err; if ( !(mbarHdl = GetNewMBar( rMBar)) ) ErrorHandler(); - SetMenuBar(mbarHdl); + SetMenuBar(mbarHdl); //dougt: if mbarHdl allocation failes above, poof. if (menuHdl = GetMenuHandle(mApple)) { @@ -240,6 +240,7 @@ void MainEventLoop(void) void ErrorHandler(void) { +//TODO: this needs to be fixed. SysBeep(10); gDone = true; } @@ -253,6 +254,7 @@ void Shutdown(void) // TO DO /* deallocate all controls */ +//dougt: check for null before deleting! DisposePtr( (char*) gControls->lw); // DisposeControl(gControls->nextB); // DisposeControl(gControls->backB); diff --git a/xpinstall/wizard/mac/src/Parser.c b/xpinstall/wizard/mac/src/Parser.c index cc951562682d..77e8f08c548a 100644 --- a/xpinstall/wizard/mac/src/Parser.c +++ b/xpinstall/wizard/mac/src/Parser.c @@ -21,9 +21,7 @@ */ -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" /*-----------------------------------------------------------* @@ -65,30 +63,35 @@ void ParseConfig(void) { OSErr err; - char **cfgText; + char *cfgText; - gControls->cfg = (Config*) NewPtrClear(sizeof(Config)); - - if(!ReadConfigFile(cfgText)) + gControls->cfg = (Config*) NewPtrClear(sizeof(Config)); +//dougt: what happens when allocation fails! + + if(!ReadConfigFile(&cfgText)) { ErrorHandler(); return; } - ERR_CHECK(PopulateLicWinKeys(*cfgText)); - ERR_CHECK(PopulateWelcWinKeys(*cfgText)); - ERR_CHECK(PopulateCompWinKeys(*cfgText)); - ERR_CHECK(PopulateSetupTypeWinKeys(*cfgText)); - ERR_CHECK(PopulateTermWinKeys(*cfgText)); - ERR_CHECK(PopulateIDIKeys(*cfgText)); + ERR_CHECK(PopulateLicWinKeys(cfgText)); + ERR_CHECK(PopulateWelcWinKeys(cfgText)); + ERR_CHECK(PopulateCompWinKeys(cfgText)); + ERR_CHECK(PopulateSetupTypeWinKeys(cfgText)); + ERR_CHECK(PopulateTermWinKeys(cfgText)); + ERR_CHECK(PopulateIDIKeys(cfgText)); - DisposePtr(*cfgText); + if (cfgText) + DisposePtr(cfgText); } Boolean ReadConfigFile(char **text) { - Boolean bSuccess = true; +//dougt: nitpick, I would have the initial bSuccess set to false, and only change to true when this function really +// succeeds. If you also set text to null from the get go, you can return the else statement during the read +// of the file. + Boolean bSuccess = true; OSErr err; FSSpec cfgFile; long dirID, dataSize; @@ -110,7 +113,8 @@ ReadConfigFile(char **text) if (dataSize > 0) { *text = (char*) NewPtrClear(dataSize); - if (err = FSRead(vRefNum, &dataSize, *text)) +//dougt: what happens when allocation fails! + if (err = FSRead(vRefNum, &dataSize, *text)) bSuccess = false; } else @@ -133,6 +137,7 @@ PopulateLicWinKeys(char *cfgText) /* LicenseWin: license file name */ gControls->cfg->licFileName = NewHandleClear(kValueMaxLen); +//dougt: what happens when allocation fails! if (!FillKeyValueUsingResID(sLicDlg, sLicFile, gControls->cfg->licFileName, cfgText)) err = eParseFailed; @@ -146,14 +151,21 @@ PopulateWelcWinKeys(char *cfgText) /* WelcomeWin: message strings */ gControls->cfg->welcMsg[0] = NewHandleClear(kValueMaxLen); +//dougt: what happens when allocation fails! if (!FillKeyValueUsingResID(sWelcDlg, sMsg0, gControls->cfg->welcMsg[0], cfgText)) - err = eParseFailed; + err = eParseFailed; //dougt: shouldn't you return now? gControls->cfg->welcMsg[1] = NewHandleClear(kValueMaxLen); - FillKeyValueUsingResID(sWelcDlg, sMsg1, gControls->cfg->welcMsg[1], cfgText); +//dougt: what happens when allocation fails! + +//dougt: why don;t you care about the error in this case? + FillKeyValueUsingResID(sWelcDlg, sMsg1, gControls->cfg->welcMsg[1], cfgText); gControls->cfg->welcMsg[2] = NewHandleClear(kValueMaxLen); - FillKeyValueUsingResID(sWelcDlg, sMsg2, gControls->cfg->welcMsg[2], cfgText); +//dougt: what happens when allocation fails! + +//dougt: why don;t you care about the error in this case? +FillKeyValueUsingResID(sWelcDlg, sMsg2, gControls->cfg->welcMsg[2], cfgText); return err; } @@ -469,8 +481,10 @@ FillKeyValueUsingSLID(short stringListID, short dlgID, short keyID, Handle dest, if (FillKeyValueUsingName(sectionName, key, dest, cfgText)) bFound = true; - DisposePtr(sectionName); - DisposePtr(key); + if(sectionName) + DisposePtr(sectionName); + if(key) + DisposePtr(key); return bFound; } @@ -484,17 +498,19 @@ FillKeyValueUsingName(char *sectionName, char *keyName, Handle dest, char *cfgTe Boolean bFound = false; value = (char*) NewPtrClear(kValueMaxLen); - +//dougt: what happens when allocation fails! if (FindKeyValue(cfgText, sectionName, keyName, value)) { HLock(dest); len = strlen(value); strncpy(*dest, value, len); +//dougt: would it be better here to do the accual allocation, PtrToHandle()? +// This way we could reduce the mem footprint by not having to allocate the max each time. HUnlock(dest); bFound = true; } - - DisposePtr(value); + if (value) + DisposePtr(value); return bFound; } @@ -508,31 +524,41 @@ FindKeyValue(const char *cfg, const char *inSectionName, const char *inKey, char sectionName = (char *) NewPtrClear( kSNameMaxLen ); section = (char *) NewPtrClear( kSectionMaxLen ); key = (char *) NewPtrClear( kKeyMaxLen ); - +//dougt: what happens when allocation fails! + /* find next section [cfgPtr moved past next section per iteration] */ - while(GetNextSection(cfgPtr, sectionName, section)) +//dougt: you are passing a pointer (cfgPtr) to a function that wants a char**. evil + while(GetNextSection(cfgPtr, sectionName, section)) { - if (strncmp(sectionName, inSectionName, strlen(inSectionName)) == 0) +//dougt: why not use strcmp here? + if (strncmp(sectionName, inSectionName, strlen(inSectionName)) == 0) { *sectionPtr = section; /* find next key [sectionPtr moved past next key per iteration] */ - while(GetNextKeyVal(sectionPtr, key, outValue)) +//dougt: you are passing a pointer (sectionPtr) to a function that wants a char**. evil + while(GetNextKeyVal(sectionPtr, key, outValue)) { +//dougt: why not use strcmp here? if (strncmp(key, inKey, strlen(key)) == 0) { - DisposePtr(key); - DisposePtr(sectionName); - DisposePtr(section); + if(key) + DisposePtr(key); + if(sectionName) + DisposePtr(sectionName); + if(section) + DisposePtr(section); return true; } } } } - - DisposePtr(key); - DisposePtr(sectionName); - DisposePtr(section); + if(key) + DisposePtr(key); + if(sectionName) + DisposePtr(sectionName); + if(section) + DisposePtr(section); return false; } @@ -671,6 +697,9 @@ GetNextKeyVal(char **inSection, char *outKey, char *outVal) * Makes a copy of the C string, converts the copy to a Pascal string, * and returns the Pascal string copy */ + +//dougt: can you use the toolbox routines? what about double bite chars? + unsigned char *CToPascal(char *str) { register char *p,*q; diff --git a/xpinstall/wizard/mac/src/SetupTypeWin.c b/xpinstall/wizard/mac/src/SetupTypeWin.c index 8069460a7804..a3e645905dc8 100644 --- a/xpinstall/wizard/mac/src/SetupTypeWin.c +++ b/xpinstall/wizard/mac/src/SetupTypeWin.c @@ -21,9 +21,8 @@ */ -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" + /*-----------------------------------------------------------* @@ -48,9 +47,10 @@ ShowSetupTypeWin(void) Str255 instLocTitle, selectFolder; GrafPtr oldPort; GetPort(&oldPort); - + //dougt: check gWPtr for null.. SetPort(gWPtr); + //dougt: change this naming scheme before I pop. gCurrWin = SETUP_TYPE; /* gControls->stw = (SetupTypeWin *) NewPtrClear(sizeof(SetupTypeWin)); */ @@ -62,13 +62,16 @@ ShowSetupTypeWin(void) gControls->stw->instDescBox = GetNewControl( rInstDescBox, gWPtr); gControls->stw->destLocBox = GetNewControl( rDestLocBox, gWPtr); gControls->stw->destLoc = GetNewControl(rDestLoc, gWPtr); + //dougt: check for failure; // populate popup button menus + //dougt: no hi HLockHi((Handle)gControls->stw->instType); pvtDataHdl = (PopupPrivateData **) (*(gControls->stw->instType))->contrlData; popupMenu = (MenuHandle) (**pvtDataHdl).mHandle; for (i=0; icfg->numSetupTypes; i++) { + //dougt: no hi. HLockHi(gControls->cfg->st[i].shortDesc); currMenuItem = CToPascal(*gControls->cfg->st[i].shortDesc); HUnlock(gControls->cfg->st[i].shortDesc); @@ -151,7 +154,7 @@ OurNavEventFunction(NavEventCallbackMessage callBackSelector, NavCBRecPtr callBa WindowPtr windowPtr; windowPtr = (WindowPtr) callBackParms->eventData.eventDataParms.event->message; - + //dougt: check for null switch(callBackSelector) { case kNavCBEvent: diff --git a/xpinstall/wizard/mac/src/TerminalWin.c b/xpinstall/wizard/mac/src/TerminalWin.c index 8cf6047fdeda..60a1319c7488 100644 --- a/xpinstall/wizard/mac/src/TerminalWin.c +++ b/xpinstall/wizard/mac/src/TerminalWin.c @@ -21,10 +21,7 @@ */ -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif - +#include "MacInstallWizard.h" /*-----------------------------------------------------------* * Terminal Window @@ -39,9 +36,10 @@ ShowTerminalWin(void) short reserr; GrafPtr oldPort; GetPort(&oldPort); - + //dougt: check for gWPtr being null SetPort(gWPtr); + //dougt: think about changing the constant to something more readable. gCurrWin = TERMINAL; /* gControls->tw = (TermWin*) NewPtrClear(sizeof(TermWin)); */ @@ -50,7 +48,7 @@ ShowTerminalWin(void) // malloc and get control rectH = Get1Resource('RECT', rStartMsgBox); - reserr = ResError(); + reserr = ResError(); //dougt: this does not do what you thing. It does not always return the last error. if (reserr == noErr) viewRect = (Rect) **((Rect **)rectH); else @@ -61,8 +59,10 @@ ShowTerminalWin(void) gControls->tw->startMsgBox = viewRect; gControls->tw->startMsg = TENew(&viewRect, &viewRect); + //dougt: check for null // populate control + //dougt: remove hi. HLockHi(gControls->cfg->startMsg); TESetText(*gControls->cfg->startMsg, strlen(*gControls->cfg->startMsg), gControls->tw->startMsg); diff --git a/xpinstall/wizard/mac/src/WelcomeWin.c b/xpinstall/wizard/mac/src/WelcomeWin.c index 3625df631186..9c2342f764cb 100644 --- a/xpinstall/wizard/mac/src/WelcomeWin.c +++ b/xpinstall/wizard/mac/src/WelcomeWin.c @@ -20,10 +20,7 @@ * Samir Gehani */ - -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" /*-----------------------------------------------------------* @@ -42,7 +39,8 @@ ShowWelcomeWin(void) GetPort(&oldPort); SetPort(gWPtr); - + +//dougt: think about changing this to a constant that is more understandable. gCurrWin = WELCOME; /* gControls->ww = (WelcWin *) NewPtrClear(sizeof(WelcWin)); */ @@ -51,9 +49,10 @@ ShowWelcomeWin(void) gControls->ww->scrollBar = GetNewControl( rLicScrollBar, gWPtr); gControls->ww->welcBox = GetNewControl( rLicBox, gWPtr); - +//dougt: you may want to do something in addition to a check for failure if(gControls->ww->scrollBar && gControls->ww->welcBox) { + //dougt: no hi. HLockHi( (Handle) gControls->ww->scrollBar); sbRect = (*(gControls->ww->welcBox))->contrlRect; @@ -86,6 +85,8 @@ InitWelcTxt(void) char * newPara; /* TE specific init */ + + //dougt: no hi. HLockHi( (Handle) gControls->ww->welcBox); viewRect = (*(gControls->ww->welcBox))->contrlRect; HUnlock( (Handle) gControls->ww->welcBox); @@ -97,14 +98,17 @@ InitWelcTxt(void) destRect.bottom = viewRect.bottom * kNumWelcScrns; /* XXX: hack */ gControls->ww->welcTxt = (TEHandle) NewPtrClear(sizeof(TEPtr)); - + //dougt: check for null + TextFont(applFont); TextFace(normal); TextSize(12); - newPara = "\r\r"; + newPara = "\r\r"; //dougt why this constant? gControls->ww->welcTxt = TENew( &destRect, &viewRect); + //dougt: check for null. + for (i=0; icfg->welcMsg[i]); @@ -143,6 +147,7 @@ InWelcomeContent(EventRecord* evt, WindowPtr wCurrPtr) case kControlDownButtonPart: case kControlPageUpPart: case kControlPageDownPart: + //dougt: this point never gets destroyed. evil scrollActionFunctionUPP = NewControlActionProc((ProcPtr) DoScrollProc); value = TrackControl(scrollBar, localPt, scrollActionFunctionUPP); return; @@ -162,7 +167,7 @@ InWelcomeContent(EventRecord* evt, WindowPtr wCurrPtr) return; break; } - + //dougt: remove the hi. HLockHi((Handle)gControls->backB); r = (**(gControls->backB)).contrlRect; HUnlock((Handle)gControls->backB); @@ -176,7 +181,8 @@ InWelcomeContent(EventRecord* evt, WindowPtr wCurrPtr) return; } } - + + //dougt: remove the hi. HLockHi((Handle)gControls->nextB); r = (**(gControls->nextB)).contrlRect; HUnlock((Handle)gControls->nextB); diff --git a/xpinstall/wizard/mac/src/XPInstallGlue.c b/xpinstall/wizard/mac/src/XPInstallGlue.c index b5cab9adedc9..11d0bc35e214 100644 --- a/xpinstall/wizard/mac/src/XPInstallGlue.c +++ b/xpinstall/wizard/mac/src/XPInstallGlue.c @@ -20,10 +20,7 @@ * Samir Gehani */ - -#ifndef _MIW_H_ - #include "MacInstallWizard.h" -#endif +#include "MacInstallWizard.h" #define XP_MAC 1 #include "xpistub.h" @@ -53,6 +50,7 @@ void xpicbStart(const char *URL, const char* UIName); void xpicbProgress(const char* msg, PRInt32 val, PRInt32 max); void xpicbFinal(const char *URL, PRInt32 finalStatus); +//dougt: has nothing to do with xpcom. Use the one in MacInstallWizard.h. #define XPCOM_ERR_CHECK(_call) \ rv = _call; \ if (NS_FAILED(rv)) \ @@ -60,7 +58,7 @@ if (NS_FAILED(rv)) \ ErrorHandler(); \ return rv; \ } - +//dougt: should be pulled from an ini file. This file may have a different name. #define XPISTUB_DLL "\pxpistubDebug.shlb"; @@ -131,17 +129,15 @@ LoadXPIStub(XPI_InitProc* pfnInit, XPI_InstallProc* pfnInstall, XPI_ExitProc* pf if (err!=noErr) return false; - // TO DO use aTargetDir to load XPISTUB_DLL + // TO DO use aTargetDir to load XPISTUB_DLL err = FSMakeFSSpec(currVRefNum, currDirID, fragName, &fslib); if (err!=noErr) return err; - - err = GetDiskFragment(&fslib, 0, kCFragGoesToEOF, nil, kPrivateCFragCopy/*kReferenceCFrag*/, connID, &mainAddr, errName); - if (err != noErr) - return err; + //dougt: verify the use of the constant kPrivateCFragCopy. + err = GetDiskFragment(&fslib, kWholeFork, kCFragGoesToEOF, nil, kPrivateCFragCopy/*kReferenceCFrag*/, connID, &mainAddr, errName); - if ( *connID != NULL) + if ( err == noErr && *connID != NULL) { ERR_CHECK_RET( FindSymbol(*connID, "\pXPI_Init", &symAddr, &symClass), err ); *pfnInit = (XPI_InitProc) symAddr; @@ -159,13 +155,13 @@ LoadXPIStub(XPI_InitProc* pfnInit, XPI_InstallProc* pfnInstall, XPI_ExitProc* pf Boolean UnloadXPIStub(CFragConnectionID* connID) { - if (*connID != NULL) +//dougt: what happens if connID is null... evil + if (*connID != NULL) { CloseConnection(connID); *connID = NULL; + return true; } - else - return false; - return true; + return false; } \ No newline at end of file