You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by da...@apache.org on 2007/07/06 02:46:53 UTC
svn commit: r553679 - /apr/apr/trunk/strings/apr_snprintf.c
Author: davi
Date: Thu Jul 5 17:46:53 2007
New Revision: 553679
URL: http://svn.apache.org/viewvc?view=rev&rev=553679
Log:
Convert wide* types to the portable apr types. Conversion table:
wide_int apr_int32_t
u_wide_int apr_uint32_t
widest_int apr_int64_t
u_widest_int apr_uint64_t
bool_int int
Passes testfmt.
Modified:
apr/apr/trunk/strings/apr_snprintf.c
Modified: apr/apr/trunk/strings/apr_snprintf.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/strings/apr_snprintf.c?view=diff&rev=553679&r1=553678&r2=553679
==============================================================================
--- apr/apr/trunk/strings/apr_snprintf.c (original)
+++ apr/apr/trunk/strings/apr_snprintf.c Thu Jul 5 17:46:53 2007
@@ -53,18 +53,6 @@
#define TRUE 1
#endif
#define NUL '\0'
-#define WIDE_INT long
-
-typedef WIDE_INT wide_int;
-typedef unsigned WIDE_INT u_wide_int;
-typedef apr_int64_t widest_int;
-#ifdef __TANDEM
-/* Although Tandem supports "long long" there is no unsigned variant. */
-typedef unsigned long u_widest_int;
-#else
-typedef apr_uint64_t u_widest_int;
-#endif
-typedef int bool_int;
#define S_NULL "(null)"
#define S_NULL_LEN 6
@@ -338,12 +326,12 @@
* (conv_10_quad), the other when we don't (conv_10). We're assuming the
* latter is faster.
*/
-static char *conv_10(register wide_int num, register bool_int is_unsigned,
- register bool_int *is_negative, char *buf_end,
+static char *conv_10(register apr_int32_t num, register int is_unsigned,
+ register int *is_negative, char *buf_end,
register apr_size_t *len)
{
register char *p = buf_end;
- register u_wide_int magnitude = num;
+ register apr_uint32_t magnitude = num;
if (is_unsigned) {
*is_negative = FALSE;
@@ -361,8 +349,8 @@
* d. add 1
*/
if (*is_negative) {
- wide_int t = num + 1;
- magnitude = ((u_wide_int) -t) + 1;
+ apr_int32_t t = num + 1;
+ magnitude = ((apr_uint32_t) -t) + 1;
}
}
@@ -370,7 +358,7 @@
* We use a do-while loop so that we write at least 1 digit
*/
do {
- register u_wide_int new_magnitude = magnitude / 10;
+ register apr_uint32_t new_magnitude = magnitude / 10;
*--p = (char) (magnitude - new_magnitude * 10 + '0');
magnitude = new_magnitude;
@@ -381,22 +369,21 @@
return (p);
}
-static char *conv_10_quad(widest_int num, register bool_int is_unsigned,
- register bool_int *is_negative, char *buf_end,
+static char *conv_10_quad(apr_int64_t num, register int is_unsigned,
+ register int *is_negative, char *buf_end,
register apr_size_t *len)
{
register char *p = buf_end;
- u_widest_int magnitude = num;
+ apr_uint64_t magnitude = num;
/*
* We see if we can use the faster non-quad version by checking the
* number against the largest long value it can be. If <=, we
* punt to the quicker version.
*/
- if ((magnitude <= ULONG_MAX && is_unsigned)
- || (num <= LONG_MAX && num >= LONG_MIN && !is_unsigned))
- return(conv_10( (wide_int)num, is_unsigned, is_negative,
- buf_end, len));
+ if ((magnitude <= UINT32_MAX && is_unsigned)
+ || (num <= INT32_MAX && num >= INT32_MIN && !is_unsigned))
+ return(conv_10(num, is_unsigned, is_negative, buf_end, len));
if (is_unsigned) {
*is_negative = FALSE;
@@ -414,8 +401,8 @@
* d. add 1
*/
if (*is_negative) {
- widest_int t = num + 1;
- magnitude = ((u_widest_int) -t) + 1;
+ apr_int64_t t = num + 1;
+ magnitude = ((apr_uint64_t) -t) + 1;
}
}
@@ -423,7 +410,7 @@
* We use a do-while loop so that we write at least 1 digit
*/
do {
- u_widest_int new_magnitude = magnitude / 10;
+ apr_uint64_t new_magnitude = magnitude / 10;
*--p = (char) (magnitude - new_magnitude * 10 + '0');
magnitude = new_magnitude;
@@ -434,13 +421,11 @@
return (p);
}
-
-
static char *conv_in_addr(struct in_addr *ia, char *buf_end, apr_size_t *len)
{
unsigned addr = ntohl(ia->s_addr);
char *p = buf_end;
- bool_int is_negative;
+ int is_negative;
apr_size_t sub_len;
p = conv_10((addr & 0x000000FF) , TRUE, &is_negative, p, &sub_len);
@@ -461,7 +446,7 @@
static char *conv_apr_sockaddr(apr_sockaddr_t *sa, char *buf_end, apr_size_t *len)
{
char *p = buf_end;
- bool_int is_negative;
+ int is_negative;
apr_size_t sub_len;
char *ipaddr_str;
@@ -529,7 +514,7 @@
* in buf).
*/
static char *conv_fp(register char format, register double num,
- boolean_e add_dp, int precision, bool_int *is_negative,
+ boolean_e add_dp, int precision, int *is_negative,
char *buf, apr_size_t *len)
{
register char *s = buf;
@@ -585,12 +570,12 @@
if (format != 'f') {
char temp[EXPONENT_LENGTH]; /* for exponent conversion */
apr_size_t t_len;
- bool_int exponent_is_negative;
+ int exponent_is_negative;
*s++ = format; /* either e or E */
decimal_point--;
if (decimal_point != 0) {
- p = conv_10((wide_int) decimal_point, FALSE, &exponent_is_negative,
+ p = conv_10((apr_int32_t) decimal_point, FALSE, &exponent_is_negative,
&temp[EXPONENT_LENGTH], &t_len);
*s++ = exponent_is_negative ? '-' : '+';
@@ -627,7 +612,7 @@
* As with conv_10, we have a faster version which is used when
* the number isn't quad size.
*/
-static char *conv_p2(register u_wide_int num, register int nbits,
+static char *conv_p2(register apr_uint32_t num, register int nbits,
char format, char *buf_end, register apr_size_t *len)
{
register int mask = (1 << nbits) - 1;
@@ -646,7 +631,7 @@
return (p);
}
-static char *conv_p2_quad(u_widest_int num, register int nbits,
+static char *conv_p2_quad(apr_uint64_t num, register int nbits,
char format, char *buf_end, register apr_size_t *len)
{
register int mask = (1 << nbits) - 1;
@@ -655,8 +640,8 @@
static const char upper_digits[] = "0123456789ABCDEF";
register const char *digits = (format == 'X') ? upper_digits : low_digits;
- if (num <= ULONG_MAX)
- return(conv_p2((u_wide_int)num, nbits, format, buf_end, len));
+ if (num <= UINT32_MAX)
+ return(conv_p2((apr_uint32_t)num, nbits, format, buf_end, len));
do {
*--p = digits[num & mask];
@@ -715,10 +700,10 @@
char prefix_char;
double fp_num;
- widest_int i_quad = (widest_int) 0;
- u_widest_int ui_quad;
- wide_int i_num = (wide_int) 0;
- u_wide_int ui_num;
+ apr_int64_t i_quad = 0;
+ apr_uint64_t ui_quad;
+ apr_int32_t i_num = 0;
+ apr_uint32_t ui_num;
char num_buf[NUM_BUF_SIZE];
char char_buf[2]; /* for printing %% and %<unknown> */
@@ -736,7 +721,7 @@
boolean_e print_blank;
boolean_e adjust_precision;
boolean_e adjust_width;
- bool_int is_negative;
+ int is_negative;
sp = vbuff->curpos;
bep = vbuff->endpos;
@@ -869,17 +854,17 @@
switch (*fmt) {
case 'u':
if (var_type == IS_QUAD) {
- i_quad = va_arg(ap, u_widest_int);
+ i_quad = va_arg(ap, apr_uint64_t);
s = conv_10_quad(i_quad, 1, &is_negative,
&num_buf[NUM_BUF_SIZE], &s_len);
}
else {
if (var_type == IS_LONG)
- i_num = (wide_int) va_arg(ap, u_wide_int);
+ i_num = (apr_int32_t) va_arg(ap, apr_uint32_t);
else if (var_type == IS_SHORT)
- i_num = (wide_int) (unsigned short) va_arg(ap, unsigned int);
+ i_num = (apr_int32_t) (unsigned short) va_arg(ap, unsigned int);
else
- i_num = (wide_int) va_arg(ap, unsigned int);
+ i_num = (apr_int32_t) va_arg(ap, unsigned int);
s = conv_10(i_num, 1, &is_negative,
&num_buf[NUM_BUF_SIZE], &s_len);
}
@@ -889,17 +874,17 @@
case 'd':
case 'i':
if (var_type == IS_QUAD) {
- i_quad = va_arg(ap, widest_int);
+ i_quad = va_arg(ap, apr_int64_t);
s = conv_10_quad(i_quad, 0, &is_negative,
&num_buf[NUM_BUF_SIZE], &s_len);
}
else {
if (var_type == IS_LONG)
- i_num = (wide_int) va_arg(ap, wide_int);
+ i_num = va_arg(ap, apr_int32_t);
else if (var_type == IS_SHORT)
- i_num = (wide_int) (short) va_arg(ap, int);
+ i_num = (short) va_arg(ap, int);
else
- i_num = (wide_int) va_arg(ap, int);
+ i_num = va_arg(ap, int);
s = conv_10(i_num, 0, &is_negative,
&num_buf[NUM_BUF_SIZE], &s_len);
}
@@ -916,17 +901,17 @@
case 'o':
if (var_type == IS_QUAD) {
- ui_quad = va_arg(ap, u_widest_int);
+ ui_quad = va_arg(ap, apr_uint64_t);
s = conv_p2_quad(ui_quad, 3, *fmt,
&num_buf[NUM_BUF_SIZE], &s_len);
}
else {
if (var_type == IS_LONG)
- ui_num = (u_wide_int) va_arg(ap, u_wide_int);
+ ui_num = va_arg(ap, apr_uint32_t);
else if (var_type == IS_SHORT)
- ui_num = (u_wide_int) (unsigned short) va_arg(ap, unsigned int);
+ ui_num = (unsigned short) va_arg(ap, unsigned int);
else
- ui_num = (u_wide_int) va_arg(ap, unsigned int);
+ ui_num = va_arg(ap, unsigned int);
s = conv_p2(ui_num, 3, *fmt,
&num_buf[NUM_BUF_SIZE], &s_len);
}
@@ -941,17 +926,17 @@
case 'x':
case 'X':
if (var_type == IS_QUAD) {
- ui_quad = va_arg(ap, u_widest_int);
+ ui_quad = va_arg(ap, apr_uint64_t);
s = conv_p2_quad(ui_quad, 4, *fmt,
&num_buf[NUM_BUF_SIZE], &s_len);
}
else {
if (var_type == IS_LONG)
- ui_num = (u_wide_int) va_arg(ap, u_wide_int);
+ ui_num = va_arg(ap, apr_uint32_t);
else if (var_type == IS_SHORT)
- ui_num = (u_wide_int) (unsigned short) va_arg(ap, unsigned int);
+ ui_num = (unsigned short) va_arg(ap, unsigned int);
else
- ui_num = (u_wide_int) va_arg(ap, unsigned int);
+ ui_num = va_arg(ap, unsigned int);
s = conv_p2(ui_num, 4, *fmt,
&num_buf[NUM_BUF_SIZE], &s_len);
}
@@ -1089,7 +1074,7 @@
case 'n':
if (var_type == IS_QUAD)
- *(va_arg(ap, widest_int *)) = cc;
+ *(va_arg(ap, apr_int64_t *)) = cc;
else if (var_type == IS_LONG)
*(va_arg(ap, long *)) = cc;
else if (var_type == IS_SHORT)
@@ -1113,14 +1098,14 @@
*/
case 'p':
#if APR_SIZEOF_VOIDP == 8
- if (sizeof(void *) <= sizeof(u_widest_int)) {
- ui_quad = (u_widest_int) va_arg(ap, void *);
+ if (sizeof(void *) <= sizeof(apr_uint64_t)) {
+ ui_quad = (apr_uint64_t) va_arg(ap, void *);
s = conv_p2_quad(ui_quad, 4, 'x',
&num_buf[NUM_BUF_SIZE], &s_len);
}
#else
- if (sizeof(void *) <= sizeof(u_wide_int)) {
- ui_num = (u_wide_int) va_arg(ap, void *);
+ if (sizeof(void *) <= sizeof(apr_uint32_t)) {
+ ui_num = (apr_uint32_t) va_arg(ap, void *);
s = conv_p2(ui_num, 4, 'x',
&num_buf[NUM_BUF_SIZE], &s_len);
}
Re: svn commit: r553679 - /apr/apr/trunk/strings/apr_snprintf.c
Posted by Davi Arnaut <da...@haxent.com.br>.
Joe Orton wrote:
> On Fri, Jul 06, 2007 at 12:46:53AM -0000, Davi Arnaut wrote:
>> Author: davi
>> Date: Thu Jul 5 17:46:53 2007
>> New Revision: 553679
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=553679
>> Log:
>> Convert wide* types to the portable apr types. Conversion table:
>>
>> wide_int apr_int32_t
>> u_wide_int apr_uint32_t
>> widest_int apr_int64_t
>> u_widest_int apr_uint64_t
>> bool_int int
>
> Are these wide_int -> int32_t changes correct? wide_int was long before
> and is now always 32-bit. Also the *INT32_MAX types are from C99
> stdint.h which might not be present.
>
Also, as I forgot to mention, there was a change of behavior with the patch:
- if ((magnitude <= ULONG_MAX && is_unsigned)
- || (num <= LONG_MAX && num >= LONG_MIN && !is_unsigned))
- return(conv_10( (wide_int)num, is_unsigned, is_negative,
- buf_end, len));
+ if ((magnitude <= UINT32_MAX && is_unsigned)
+ || (num <= INT32_MAX && num >= INT32_MIN && !is_unsigned))
+ return(conv_10(num, is_unsigned, is_negative, buf_end, ...
On a 64 bits build the conv_10 optimization could be inadvertently used
because ULONG_MAX would be 64-bit large, but since wide_int was `long`
it worked. Now with the behavior fix, conv_10 is only called on values
that really fits on 32 bits.
--
Davi Arnaut
Re: svn commit: r553679 - /apr/apr/trunk/strings/apr_snprintf.c
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Jul 06, 2007 at 08:51:38AM -0300, Davi Arnaut wrote:
> Joe Orton wrote:
> > Are these wide_int -> int32_t changes correct? wide_int was long before
> > and is now always 32-bit.
>
> When I made the patch, it seemed to be correct because the format checks
> using IS_LONG were based upon APR_INT64_T_FMT.
Ah, OK. It looks a bit counter-intuitive but it seems to work OK, so no
problem.
> > Also the *INT32_MAX types are from C99 stdint.h which might not be present.
>
> Good catch, I guess we could use *INT_MAX. This begs the question: why
> don't we have APR_*INT32_MIN/MAX? Should be pretty straight forward:
It's not the first time the question has been asked... no objection to
adding such anyway. Note the private definition in apr_private_common.h
would need to be removed.
> defaults:
>
> #define APR_UINT32_MAX 0xffffffffU
> #define APR_UINT64_MAX 0xffffffffffffffffULL
>
> autoconf logic:
Having autoconf tests to calculate values which are mathematical
constants seems like... overkill :) I would just add the #defines as
above; at most doing:
#ifdef APR_HAVE_STDINT_H
#define APR_UINT32_MAX UINT32_MAX
etc
#else
#define APR_UINT32_MAX 0xetc
etc
note using APR_*INT64_C() for the 64-bit constants is a good idea.
joe
Re: svn commit: r553679 - /apr/apr/trunk/strings/apr_snprintf.c
Posted by Davi Arnaut <da...@haxent.com.br>.
Martin Kraemer wrote:
> On Mon, Jul 09, 2007 at 10:22:54AM -0300, Davi Arnaut wrote:
>> Could you try the attached patch? Thanks.
>
> Okay -- the patch fixes it for FreeBSD-4.11
>
> Thanks for your patch,
>
Ok, thanks. I'm commit the patch in a few minutes.
--
Davi Arnaut
Re: svn commit: r553679 - /apr/apr/trunk/strings/apr_snprintf.c
Posted by Davi Arnaut <da...@haxent.com.br>.
Martin Kraemer wrote:
> On Fri, Jul 06, 2007 at 08:51:38AM -0300, Davi Arnaut wrote:
>>> Also the *INT32_MAX types are from C99 stdint.h which might not be present.
>> Good catch, I guess we could use *INT_MAX. This begs the question: why
>> don't we have APR_*INT32_MIN/MAX?
>
> Good point. Especially for the apr-defined int types, there should
> also be a <type>_MAX define.
>
> I'm +1 for adding them -- as at the moment, on FreeBSD-4.x I cannot compile apr:
>
> /usr/local/bin/bash /home/martin/apachen/X/httpd-2.3/srclib/apr/libtool --silent --mode=compile gcc -g -O2 -Wall -DHAVE_CONFIG_H -D_THREAD_SAFE -D_REENTRANT -I./include -I/home/martin/apachen/X/httpd-2.3/srclib/apr/include/arch/unix -I./include/arch/unix -I/home/martin/apachen/X/httpd-2.3/srclib/apr/include -o strings/apr_snprintf.lo -c strings/apr_snprintf.c && touch strings/apr_snprintf.lo
> strings/apr_snprintf.c: In function `conv_10_quad':
> strings/apr_snprintf.c:384: `UINT32_MAX' undeclared (first use in this function)
> strings/apr_snprintf.c:384: (Each undeclared identifier is reported only once
> strings/apr_snprintf.c:384: for each function it appears in.)
> strings/apr_snprintf.c:385: `INT32_MAX' undeclared (first use in this function)
> strings/apr_snprintf.c:385: `INT32_MIN' undeclared (first use in this function)
> strings/apr_snprintf.c: In function `conv_p2_quad':
> strings/apr_snprintf.c:643: `UINT32_MAX' undeclared (first use in this function)
> Make[1]: *** [strings/apr_snprintf.lo] Error 1
> Make[1]: Leaving directory `/home/martin/apachen/X/httpd-2.3/srclib/apr'
>
> and there's no <stdint.h>, and neither <inttypes.h> nor <limits.h>
> provide something like *INT32_MAX.
>
Could you try the attached patch? Thanks.
--
Davi Arnaut
Re: svn commit: r553679 - /apr/apr/trunk/strings/apr_snprintf.c
Posted by Martin Kraemer <Ma...@Fujitsu-Siemens.com>.
On Fri, Jul 06, 2007 at 08:51:38AM -0300, Davi Arnaut wrote:
>
> > Also the *INT32_MAX types are from C99 stdint.h which might not be present.
>
> Good catch, I guess we could use *INT_MAX. This begs the question: why
> don't we have APR_*INT32_MIN/MAX?
Good point. Especially for the apr-defined int types, there should
also be a <type>_MAX define.
I'm +1 for adding them -- as at the moment, on FreeBSD-4.x I cannot compile apr:
/usr/local/bin/bash /home/martin/apachen/X/httpd-2.3/srclib/apr/libtool --silent --mode=compile gcc -g -O2 -Wall -DHAVE_CONFIG_H -D_THREAD_SAFE -D_REENTRANT -I./include -I/home/martin/apachen/X/httpd-2.3/srclib/apr/include/arch/unix -I./include/arch/unix -I/home/martin/apachen/X/httpd-2.3/srclib/apr/include -o strings/apr_snprintf.lo -c strings/apr_snprintf.c && touch strings/apr_snprintf.lo
strings/apr_snprintf.c: In function `conv_10_quad':
strings/apr_snprintf.c:384: `UINT32_MAX' undeclared (first use in this function)
strings/apr_snprintf.c:384: (Each undeclared identifier is reported only once
strings/apr_snprintf.c:384: for each function it appears in.)
strings/apr_snprintf.c:385: `INT32_MAX' undeclared (first use in this function)
strings/apr_snprintf.c:385: `INT32_MIN' undeclared (first use in this function)
strings/apr_snprintf.c: In function `conv_p2_quad':
strings/apr_snprintf.c:643: `UINT32_MAX' undeclared (first use in this function)
Make[1]: *** [strings/apr_snprintf.lo] Error 1
Make[1]: Leaving directory `/home/martin/apachen/X/httpd-2.3/srclib/apr'
and there's no <stdint.h>, and neither <inttypes.h> nor <limits.h>
provide something like *INT32_MAX.
Martin
--
<Ma...@Fujitsu-Siemens.com> | Fujitsu Siemens
http://www.fujitsu-siemens.com/imprint.html | 81730 Munich, Germany
Re: svn commit: r553679 - /apr/apr/trunk/strings/apr_snprintf.c
Posted by Davi Arnaut <da...@haxent.com.br>.
Joe Orton wrote:
> On Fri, Jul 06, 2007 at 12:46:53AM -0000, Davi Arnaut wrote:
>> Author: davi
>> Date: Thu Jul 5 17:46:53 2007
>> New Revision: 553679
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=553679
>> Log:
>> Convert wide* types to the portable apr types. Conversion table:
>>
>> wide_int apr_int32_t
>> u_wide_int apr_uint32_t
>> widest_int apr_int64_t
>> u_widest_int apr_uint64_t
>> bool_int int
>
> Are these wide_int -> int32_t changes correct? wide_int was long before
> and is now always 32-bit.
When I made the patch, it seemed to be correct because the format checks
using IS_LONG were based upon APR_INT64_T_FMT.
> Also the *INT32_MAX types are from C99 stdint.h which might not be present.
Good catch, I guess we could use *INT_MAX. This begs the question: why
don't we have APR_*INT32_MIN/MAX? Should be pretty straight forward:
defaults:
#define APR_UINT32_MAX 0xffffffffU
#define APR_UINT64_MAX 0xffffffffffffffffULL
autoconf logic:
try to figure out from limits.h [1]
try to figure out from stdint.h
if none worked, use defaults.
1:
#include <limits.h>
#define stringify(x) #x
#define to_string(x) stringify(x)
int main()
{
/* also check if different from default */
printf("%s\n", to_string(ULONG_MAX));
return 0;
}
Ok, that's pretty creative.. but should work.
--
Davi Arnaut
Re: svn commit: r553679 - /apr/apr/trunk/strings/apr_snprintf.c
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Jul 06, 2007 at 12:46:53AM -0000, Davi Arnaut wrote:
> Author: davi
> Date: Thu Jul 5 17:46:53 2007
> New Revision: 553679
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=553679
> Log:
> Convert wide* types to the portable apr types. Conversion table:
>
> wide_int apr_int32_t
> u_wide_int apr_uint32_t
> widest_int apr_int64_t
> u_widest_int apr_uint64_t
> bool_int int
Are these wide_int -> int32_t changes correct? wide_int was long before
and is now always 32-bit. Also the *INT32_MAX types are from C99
stdint.h which might not be present.
...
> if (var_type == IS_LONG)
> - ui_num = (u_wide_int) va_arg(ap, u_wide_int);
> + ui_num = va_arg(ap, apr_uint32_t);