Bug 1402151, part 2 - Clamp the number of blocks for a VMS file. r=michal
strtoul returns an |unsigned long|, and clamps the return value to
ULONG_MAX if the input is out of range. This means that the value
returned for large numbers differs between 32-bit and 64-bit
systems. To make testing easier, this patch clamps the return value to
UINT32_MAX, ensuring that 32-bit and 64-bit builds of Firefox will be
consistent.
Explicitly declaring that the intermediate value numBlocks is a
uint64_t also avoids (well-defined) integer overflow in the case of a
large file.
This patch also changes PRId64 to PRIu64, because the value being
printed is unsigned. However, this should not make a difference in
practice, because, with clamping, the maximum value being printed is
|UINT32_MAX * 512|.
I also cleaned up the comment that was a little garbled and contained
information about both the old and new ways this was handled, and
removed some code that has been ifdef'd out for at least 14 years.
MozReview-Commit-ID: HELmWmtx24O
--- a/netwerk/streamconv/converters/ParseFTPList.cpp
+++ b/netwerk/streamconv/converters/ParseFTPList.cpp
@@ -1,14 +1,15 @@
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "ParseFTPList.h"
+#include <algorithm>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include "plstr.h"
#include "nsDebug.h"
#include "prprf.h"
#include "mozilla/CheckedInt.h"
#include "mozilla/IntegerPrintfMacros.h"
@@ -461,56 +462,27 @@ int ParseFTPList(const char *line, struc
* 'used' is the size in bytes if and only if 'used'<=allocated.
* If 'used' is size in bytes then it can be > 2^32
* If 'used' is not size in bytes then it is size in blocks.
*/
pos = 0;
while (pos < toklen[1] && (tokens[1][pos]) != '/')
pos++;
-/*
- * I've never seen size come back in bytes, its always in blocks, and
- * the following test fails. So, always perform the "size in blocks".
- * I'm leaving the "size in bytes" code if'd out in case we ever need
- * to re-instate it.
-*/
-#if 0
- if (pos < toklen[1] && ( (pos<<1) > (toklen[1]-1) ||
- (strtoul(tokens[1], (char **)0, 10) >
- strtoul(tokens[1]+pos+1, (char **)0, 10)) ))
- { /* size is in bytes */
- if (pos > (sizeof(result->fe_size)-1))
- pos = sizeof(result->fe_size)-1;
- memcpy( result->fe_size, tokens[1], pos );
- result->fe_size[pos] = '\0';
- }
- else /* size is in blocks */
-#endif
- {
- /* size requires multiplication by blocksize.
- *
- * We could assume blocksize is 512 (like Lynx does) and
- * shift by 9, but that might not be right. Even if it
- * were, doing that wouldn't reflect what the file's
- * real size was. The sanest thing to do is not use the
- * LISTing's filesize, so we won't (like ftpmirror).
- *
- * ulltoa(((unsigned long long)fsz)<<9, result->fe_size, 10);
- *
- * A block is always 512 bytes on OpenVMS, compute size.
- * So its rounded up to the next block, so what, its better
- * than not showing the size at all.
- * A block is always 512 bytes on OpenVMS, compute size.
- * So its rounded up to the next block, so what, its better
- * than not showing the size at all.
- */
- uint64_t fsz = uint64_t(strtoul(tokens[1], (char **)0, 10) * 512);
- SprintfLiteral(result->fe_size, "%" PRId64, fsz);
- }
-
+ /*
+ * On OpenVMS, the size is given in blocks. A block is 512
+ * bytes. This can only approximate the size of the file,
+ * but that's better than not showing a size at all.
+ * numBlocks is clamped to UINT32_MAX to make 32-bit and
+ * 64-bit builds return consistent results.
+ */
+ uint64_t numBlocks = strtoul(tokens[1], nullptr, 10);
+ numBlocks = std::min(numBlocks, (uint64_t)UINT32_MAX);
+ uint64_t fileSize = numBlocks * 512;
+ SprintfLiteral(result->fe_size, "%" PRIu64, fileSize);
} /* if (result->fe_type != 'd') */
p = tokens[2] + 2;
if (*p == '-')
p++;
tbuf[0] = p[0];
tbuf[1] = tolower(p[1]);
tbuf[2] = tolower(p[2]);