On Mar 10, 2010, at 5:50 PM, William A. Rowe Jr. wrote:
> On 3/10/2010 4:45 PM, Hyrum K. Wright wrote:
>>
>> Digging deeper, it appears to be an error in apr_vformatter() when parsing
>> the format '%lld'. I'm running the tests on Mac OS X where APR_OFF_FMT_T is
>> defined as lld, so this format occurs frequently. Consequently, I'm also
>> seeing a failure in testfmt at line 63, where the parser is attempting to
>> parse APR_OFF_FMT_T, but failing. Can anybody else replicate this bug?
>
> Yes, this function isn't maintained on Mac OS/X. Which is something
> of a surprise, given the number of Mac fans around here!
>
> The defintions are very clear, %d must handle an (int) sized object,
> %ld must handle a (long) sized object, and %lld must handle any
> long long int sized object. Patches welcome.
>
> I plan to patch win32 to accept %I64[ld] because long long type was added
> with VS 7.1, while %ll was not added until the more recent Visual Studio 8.
> I64d goes further back in time to 6.0 or prior, and is how we specified
> the format tag for APR_OFF_T_FMT etc.
Digging even further. It appears that the parser is attempting to match
APR_INT64_T_FMT first, which may be of variable size. On OS X, this is defined
as 'ld', but the parser mistakenly matches APR_OFF_T_FMT, which is defined as
'lld' for this case.
Attached is a patch which adds another block which preemptively matches
APR_OFF_T_FMT. I've tested it on OS X and a 64-bit Linux system.
-Hyrum
Index: strings/apr_snprintf.c
===================================================================
--- strings/apr_snprintf.c (revision 921388)
+++ strings/apr_snprintf.c (working copy)
@@ -810,10 +810,23 @@ APR_DECLARE(int) apr_vformatter(int (*flush_func)(
adjust_precision = adjust_width = NO;
/*
- * Modifier check. Note that if APR_INT64_T_FMT is "d",
- * the first if condition is never true.
+ * Modifier check. Note that if APR_OFF_T_FMT is "d",
+ * the first if condition is never true, and if APR_INT64_T_FMT
+ * is "d", the second if condition is never true.
*/
- if ((sizeof(APR_INT64_T_FMT) == 4 &&
+ if ((sizeof(APR_OFF_T_FMT) == 4 &&
+ fmt[0] == APR_OFF_T_FMT[0] &&
+ fmt[1] == APR_OFF_T_FMT[1]) ||
+ (sizeof(APR_OFF_T_FMT) == 3 &&
+ fmt[0] == APR_OFF_T_FMT[0]) ||
+ (sizeof(APR_OFF_T_FMT) > 4 &&
+ strncmp(fmt, APR_OFF_T_FMT,
+ sizeof(APR_OFF_T_FMT) - 2) == 0)) {
+ /* Need to account for trailing 'd' and null in sizeof() */
+ var_type = IS_QUAD;
+ fmt += (sizeof(APR_OFF_T_FMT) - 2);
+ }
+ else if ((sizeof(APR_INT64_T_FMT) == 4 &&
fmt[0] == APR_INT64_T_FMT[0] &&
fmt[1] == APR_INT64_T_FMT[1]) ||
(sizeof(APR_INT64_T_FMT) == 3 &&
