xpcom_obsolete was removed from libxul in bug 332114. The firefox-on-xulrunner build fails on nsNetscapeProfileMigratorBase.cpp because nsIRegistry.h is no longer avaliable.

Heh, I didn't know anyone but me was building that configuration. I was planning on replacing the nsIRegistry usage with NSReg.h calls.

I have patch for this. I will clean it a bit, and send it tomorow.

This patch converts nsIRegistry* usage to libreg API.

Comment on attachment 218413 [details] [diff] [review] migrator fix v1 >Index: browser/components/migration/src/nsNetscapeProfileMigratorBase.cpp >+static char *GetLongStringFromReg(HREG reg, RKEY key, char *name) >+{ >+ REGERR errCode; >+ PRUint32 strSize = MAXREGPATHLEN; >+ char *str = nsnull; >+ >+ do { >+ nsMemory::Free(str); >+ >+ if (!(str = (char*)nsMemory::Alloc(strSize))) >+ return nsnull; >+ >+ errCode = NR_RegGetEntryString(reg, key, name, str, strSize); >+ >+ strSize *= 2; >+ } while (errCode == REGERR_BUFTOOSMALL); >+ >+ if (errCode) { >+ nsMemory::Free(str); >+ return nsnull; >+ } >+ return str; >+} 1) This signature is bad. Returning an alloc'ed string is not a good idea, because it's not immediately clear to callers whether (and with which allocator) they're supposed to free the pointer; why not make this signature: nsresult GetLongStringFromReg(HREG reg, RKEY key, char *name, nsACString& aResult); This signature also allows you to simplify a lot of the code below, which has buffers and conditional frees of longstr strewn about. 2) this looping construct is unnecessary. NR_RegGetEntryInfo will give you the exact length you need to allocate. 3) If you're going to use the XPCOM allocator, please use NS_Alloc and NS_Free instead of the old nsMemory:: aliases. > nsresult > nsNetscapeProfileMigratorBase::GetProfileDataFromRegistry(nsILocalFile* aRegistryFile, > nsISupportsArray* aProfileNames, > nsISupportsArray* aProfileLocations) >+cleanup4: >+ if (str != longStr) >+ nsMemory::Free(longStr); >+ >+cleanup3: >+ NR_RegClose(reg); >+ >+cleanup2: >+ NR_ShutdownRegistry(); >+ >+ if (NS_FAILED(rv)) >+ return rv; >+ >+cleanup1: >+ switch (errCode) { >+ case REGERR_PARAM: >+ case REGERR_BADTYPE: >+ case REGERR_BADNAME: >+ return NS_ERROR_INVALID_ARG; >+ >+ case REGERR_MEMORY: >+ return NS_ERROR_OUT_OF_MEMORY; > } >- return rv; >+ return NS_ERROR_FAILURE; > } I am not absolutely opposed to gotos, but in this case I think they complicate the codeflow unnecessarily. Can you restructure this using either nested ifs or subroutines?

I hope this version you will like more. I removed that complex buffer resize code (i didn't know that you can get buffer size upfront). I also simplified cleanup code.

Comment on attachment 218470 [details] [diff] [review] migrator fix v2 Excellent! Let me know if you need me to land this.

Fixed on trunk.