Bug 597630 - Fix __snprintf / snwprintf usage in updater.cpp and various x64 compiler warnings. r-dolske, a=approval2.0

toolkit/mozapps/readstrings/readstrings.cpp

@@ -179,7 +179,7 @@ ReadStrings(const NS_tchar *path,
   if (fseek(fp, 0, SEEK_SET) != 0)
     return READ_ERROR;
 
-  int rd = fread(fileContents, sizeof(char), flen, fp);
+  size_t rd = fread(fileContents, sizeof(char), flen, fp);
   if (rd != flen)
     return READ_ERROR;
 

toolkit/mozapps/update/updater/bspatch.cpp

@@ -62,7 +62,7 @@
 int
 MBS_ReadHeader(FILE* file, MBSPatchHeader *header)
 {
-  int s = fread(header, 1, sizeof(MBSPatchHeader), file);
+  size_t s = fread(header, 1, sizeof(MBSPatchHeader), file);
   if (s != sizeof(MBSPatchHeader))
     return READ_ERROR;
 
@@ -104,10 +104,10 @@ MBS_ApplyPatch(const MBSPatchHeader *header, FILE* patchFile,
 
   int rv = OK;
 
-  int r = header->cblen + header->difflen + header->extralen;
+  size_t r = header->cblen + header->difflen + header->extralen;
   unsigned char *wb = buf;
   while (r) {
-    int c = fread(wb, 1, (r > SSIZE_MAX) ? SSIZE_MAX : r, patchFile);
+    size_t c = fread(wb, 1, (r > SSIZE_MAX) ? SSIZE_MAX : r, patchFile);
     if (c < 0) {
       rv = READ_ERROR;
       goto end;

toolkit/mozapps/update/updater/progressui_win.cpp

@@ -22,6 +22,7 @@
  * Contributor(s):
  *  Darin Fisher <darin@meer.net>
  *  Masayuki Nakano <masayuki@d-toybox.com>
+ *  Robert Strong <robert.bugzilla@gmail.com>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -153,10 +154,10 @@ InitDialog(HWND hDlg)
   WCHAR szwTitle[MAX_TEXT_LEN];
   WCHAR szwInfo[MAX_TEXT_LEN];
 
-  MultiByteToWideChar(CP_UTF8, 0, uiStrings.title, strlen(uiStrings.title) + 1,
-                      szwTitle, sizeof(szwTitle)/sizeof(szwTitle[0]));
-  MultiByteToWideChar(CP_UTF8, 0, uiStrings.info, strlen(uiStrings.info) + 1,
-                      szwInfo, sizeof(szwInfo)/sizeof(szwInfo[0]));
+  MultiByteToWideChar(CP_UTF8, 0, uiStrings.title, -1, szwTitle,
+                      sizeof(szwTitle)/sizeof(szwTitle[0]));
+  MultiByteToWideChar(CP_UTF8, 0, uiStrings.info, -1, szwInfo,
+                      sizeof(szwInfo)/sizeof(szwInfo[0]));
 
   SetWindowTextW(hDlg, szwTitle);
   SetWindowTextW(GetDlgItem(hDlg, IDC_INFO), szwInfo);
@@ -349,6 +350,11 @@ ShowProgressUI()
     return -1;
   if (_waccess(filename, 04))
     return -1;
+  // If the updater.ini doesn't have the required strings, then we should not
+  // bother showing UI.
+  StringTable uiStrings;
+  if (ReadStrings(filename, &uiStrings) != OK)
+    return -1;
 
   INITCOMMONCONTROLSEX icc = {
     sizeof(INITCOMMONCONTROLSEX),

toolkit/mozapps/update/updater/updater.cpp

@@ -59,11 +59,26 @@
 #ifndef WINCE
 # define putenv _putenv
 #endif
-# define snprintf _snprintf
 # define fchmod(a,b)
-
 # define NS_T(str) L ## str
-# define NS_tsnprintf _snwprintf
+// On Windows, _snprintf and _snwprintf don't guarantee null termination. These
+// macros always leave room in the buffer for null termination and set the end
+// of the buffer to null in case the string is larger than the buffer. Having
+// multiple nulls in a string is fine and this approach is simpler (possibly
+// faster) than calculating the string length to place the null terminator and
+// will truncate the string as _snprintf and _snwprintf do on other platforms.
+# define snprintf(dest, count, fmt, ...) \
+  PR_BEGIN_MACRO \
+    int _count = count - 1; \
+    _snprintf(dest, _count, fmt, ##__VA_ARGS__); \
+    dest[_count] = '\0'; \
+  PR_END_MACRO
+# define NS_tsnprintf(dest, count, fmt, ...) \
+  PR_BEGIN_MACRO \
+    int _count = count - 1; \
+    _snwprintf(dest, _count, fmt, ##__VA_ARGS__); \
+    dest[_count] = L'\0'; \
+  PR_END_MACRO
 # define NS_tstrrchr wcsrchr
 # define NS_taccess _waccess
 # define NS_tchdir _wchdir
@@ -71,7 +86,6 @@
 # define NS_tmkdir(path, perms) _wmkdir(path)
 # define NS_tremove _wremove
 # define NS_tfopen _wfopen
-# define NS_tatoi _wtoi64
 #ifndef WINCE
 # define stat _stat
 #endif
@@ -92,7 +106,6 @@
 # define NS_tmkdir mkdir
 # define NS_tremove remove
 # define NS_tfopen fopen
-# define NS_tatoi atoi
 # define NS_tstat stat
 # define BACKUP_EXT ".moz-backup"
 # define LOG_S "%s"
@@ -371,7 +384,8 @@ static void LogInit()
     return;
 
   NS_tchar logFile[MAXPATHLEN];
-  NS_tsnprintf(logFile, MAXPATHLEN, NS_T("%s/update.log"), gSourcePath);
+  NS_tsnprintf(logFile, sizeof(logFile)/sizeof(logFile[0]),
+               NS_T("%s/update.log"), gSourcePath);
 
   gLogFP = NS_tfopen(logFile, NS_T("w"));
 }
@@ -566,9 +580,9 @@ static int copy_file(const NS_tchar *spath, const NS_tchar *dpath)
     return WRITE_ERROR;
   }
 
-  int sc;
+  size_t sc;
   while ((sc = fread(BigBuffer, 1, BigBufferSize, sfile)) > 0) {
-    int dc;
+    size_t dc;
     char *bp = BigBuffer;
     while ((dc = fwrite(bp, 1, (unsigned int) sc, dfile)) > 0) {
       if ((sc -= dc) == 0)
@@ -594,7 +608,8 @@ static int copy_file(const NS_tchar *spath, const NS_tchar *dpath)
 static int backup_create(const NS_tchar *path)
 {
   NS_tchar backup[MAXPATHLEN];
-  NS_tsnprintf(backup, sizeof(backup), NS_T("%s" BACKUP_EXT), path);
+  NS_tsnprintf(backup, sizeof(backup)/sizeof(backup[0]),
+               NS_T("%s" BACKUP_EXT), path);
 
   return copy_file(path, backup);
 }
@@ -605,7 +620,8 @@ static int backup_create(const NS_tchar *path)
 static int backup_restore(const NS_tchar *path)
 {
   NS_tchar backup[MAXPATHLEN];
-  NS_tsnprintf(backup, sizeof(backup), NS_T("%s" BACKUP_EXT), path);
+  NS_tsnprintf(backup, sizeof(backup)/sizeof(backup[0]),
+               NS_T("%s" BACKUP_EXT), path);
 
   int rv = copy_file(backup, path);
   if (rv) {
@@ -627,7 +643,8 @@ static int backup_restore(const NS_tchar *path)
 static int backup_discard(const NS_tchar *path)
 {
   NS_tchar backup[MAXPATHLEN];
-  NS_tsnprintf(backup, sizeof(backup), NS_T("%s" BACKUP_EXT), path);
+  NS_tsnprintf(backup, sizeof(backup)/sizeof(backup[0]),
+               NS_T("%s" BACKUP_EXT), path);
 
   int rv = ensure_remove(backup);
   if (rv)
@@ -928,10 +945,10 @@ PatchFile::LoadSourceFile(FILE* ofile)
   if (!buf)
     return MEM_ERROR;
 
-  int r = header.slen;
+  size_t r = header.slen;
   unsigned char *rb = buf;
   while (r) {
-    int c = fread(rb, 1, r, ofile);
+    size_t c = fread(rb, 1, r, ofile);
     if (c < 0) {
       LOG(("LoadSourceFile: error reading destination file: " LOG_S "\n",
            mDestFile));
@@ -996,8 +1013,8 @@ PatchFile::Prepare()
   // extract the patch to a temporary file
   mPatchIndex = sPatchIndex++;
 
-  NS_tsnprintf(spath, MAXPATHLEN, NS_T("%s/%d.patch"),
-               gSourcePath, mPatchIndex);
+  NS_tsnprintf(spath, sizeof(spath)/sizeof(spath[0]),
+               NS_T("%s/%d.patch"), gSourcePath, mPatchIndex);
 
   NS_tremove(spath);
 
@@ -1252,12 +1269,13 @@ LaunchWinPostProcess(const WCHAR *appExe)
   wcscpy(slash + 1, L"uninstall.update");
 
   WCHAR slogFile[MAXPATHLEN];
-  _snwprintf(slogFile, MAXPATHLEN, L"%s/update.log", gSourcePath);
+  NS_tsnprintf(slogFile, sizeof(slogFile)/sizeof(slogFile[0]),
+               NS_T("%s/update.log"), gSourcePath);
 
   WCHAR dummyArg[13];
   wcscpy(dummyArg, L"argv0ignored ");
 
-  int len = wcslen(exearg) + wcslen(dummyArg);
+  size_t len = wcslen(exearg) + wcslen(dummyArg);
   WCHAR *cmdline = (WCHAR *) malloc((len + 1) * sizeof(WCHAR));
   if (!cmdline)
     return;
@@ -1378,7 +1396,8 @@ WriteStatusFile(int status)
   // This is how we communicate our completion status to the main application.
 
   NS_tchar filename[MAXPATHLEN];
-  NS_tsnprintf(filename, MAXPATHLEN, NS_T("%s/update.status"), gSourcePath);
+  NS_tsnprintf(filename, sizeof(filename)/sizeof(filename[0]),
+               NS_T("%s/update.status"), gSourcePath);
 
   AutoFile file = NS_tfopen(filename, NS_T("wb+"));
   if (file == NULL)
@@ -1390,7 +1409,7 @@ WriteStatusFile(int status)
   if (status == OK) {
     text = "succeeded\n";
   } else {
-    snprintf(buf, sizeof(buf), "failed: %d\n", status);
+    snprintf(buf, sizeof(buf)/sizeof(buf[0]), "failed: %d\n", status);
     text = buf;
   }
   fwrite(text, strlen(text), 1, file);
@@ -1402,7 +1421,8 @@ UpdateThreadFunc(void *param)
   // open ZIP archive and process...
 
   NS_tchar dataFile[MAXPATHLEN];
-  NS_tsnprintf(dataFile, MAXPATHLEN, NS_T("%s/update.mar"), gSourcePath);
+  NS_tsnprintf(dataFile, sizeof(dataFile)/sizeof(dataFile[0]),
+               NS_T("%s/update.mar"), gSourcePath);
 
   int rv = gArchiveReader.Open(dataFile);
   if (rv == OK) {
@@ -1443,7 +1463,11 @@ int NS_main(int argc, NS_tchar **argv)
 #endif
 
   if (argc > 2 ) {
-    int pid = NS_tatoi(argv[2]);
+#ifdef XP_WIN
+    __int64 pid = _wtoi64(argv[2]);
+#else
+    int pid = atoi(argv[2]);
+#endif
     if (pid) {
 #ifdef XP_WIN
       HANDLE parent = OpenProcess(SYNCHRONIZE, FALSE, (DWORD) pid);
@@ -1481,7 +1505,8 @@ int NS_main(int argc, NS_tchar **argv)
   NS_tchar elevatedLockFilePath[MAXPATHLEN];
   if (argc > argOffset) {
     NS_tchar updateLockFilePath[MAXPATHLEN];
-    NS_tsnprintf(updateLockFilePath, MAXPATHLEN,
+    NS_tsnprintf(updateLockFilePath,
+                 sizeof(updateLockFilePath)/sizeof(updateLockFilePath[0]),
                  NS_T("%s.update_in_progress.lock"), argv[argOffset]);
 
     // The update_in_progress.lock file should only exist during an update. In
@@ -1501,7 +1526,8 @@ int NS_main(int argc, NS_tchar **argv)
                                        FILE_FLAG_DELETE_ON_CLOSE,
                                        NULL);
 
-    NS_tsnprintf(elevatedLockFilePath, MAXPATHLEN,
+    NS_tsnprintf(elevatedLockFilePath,
+                 sizeof(elevatedLockFilePath)/sizeof(elevatedLockFilePath[0]),
                  NS_T("%s/update_elevated.lock"), argv[1]);
 
     if (updateLockFileHandle == INVALID_HANDLE_VALUE) {
@@ -1598,7 +1624,8 @@ int NS_main(int argc, NS_tchar **argv)
     FindClose(hFindFile);
 
     // Make a copy of the callback executable.
-    NS_tsnprintf(callbackBackupPath, sizeof(callbackBackupPath),
+    NS_tsnprintf(callbackBackupPath,
+                 sizeof(callbackBackupPath)/sizeof(callbackBackupPath[0]),
                  NS_T("%s" CALLBACK_BACKUP_EXT), argv[argOffset]);
     NS_tremove(callbackBackupPath);
     CopyFileW(argv[argOffset], callbackBackupPath, FALSE);
@@ -1813,7 +1840,8 @@ ActionList::Finish(int status)
 int DoUpdate()
 {
   NS_tchar manifest[MAXPATHLEN];
-  NS_tsnprintf(manifest, MAXPATHLEN, NS_T("%s/update.manifest"), gSourcePath);
+  NS_tsnprintf(manifest, sizeof(manifest)/sizeof(manifest[0]),
+               NS_T("%s/update.manifest"), gSourcePath);
 
   // extract the manifest
   FILE *fp = NS_tfopen(manifest, NS_T("wb"));
@@ -1846,10 +1874,10 @@ int DoUpdate()
   if (!mbuf)
     return MEM_ERROR;
 
-  int r = ms.st_size;
+  size_t r = ms.st_size;
   char *rb = mbuf;
   while (r) {
-    int c = fread(rb, 1, mmin(SSIZE_MAX,r), mfile);
+    size_t c = fread(rb, 1, mmin(SSIZE_MAX,r), mfile);
     if (c < 0) {
       LOG(("DoUpdate: error reading manifest file: " LOG_S "\n", manifest));
       return READ_ERROR;