Skip to content

Commit

Permalink
date module, replacing abs call with the llabs's like one due to bigg…
Browse files Browse the repository at this point in the history 
…er type
  • Loading branch information
@devnexen @krakjoe
devnexen authored and krakjoe committed Nov 7, 2017
1 parent f70ca77 commit c189845
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions ext/date/php_date.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2290,8 +2290,8 @@ static HashTable *date_object_get_properties(zval *object) /* {{{ */

ZSTR_LEN(tmpstr) = snprintf(ZSTR_VAL(tmpstr), sizeof("+05:00"), "%c%02d:%02d",
utc_offset < 0 ? '-' : '+',
abs(utc_offset / 3600),
abs(((utc_offset % 3600) / 60)));
php_date_llabs(utc_offset / 3600),
php_date_llabs(((utc_offset % 3600) / 60)));

ZVAL_NEW_STR(&zv, tmpstr);
}
Expand Down Expand Up @@ -2382,8 +2382,8 @@ static HashTable *date_object_get_properties_timezone(zval *object) /* {{{ */

ZSTR_LEN(tmpstr) = snprintf(ZSTR_VAL(tmpstr), sizeof("+05:00"), "%c%02d:%02d",
tzobj->tzi.utc_offset < 0 ? '-' : '+',
abs(tzobj->tzi.utc_offset / 3600),
abs(((tzobj->tzi.utc_offset % 3600) / 60)));
php_date_llabs(tzobj->tzi.utc_offset / 3600),
php_date_llabs(((tzobj->tzi.utc_offset % 3600) / 60)));

ZVAL_NEW_STR(&zv, tmpstr);
}
Expand Down Expand Up @@ -3912,8 +3912,8 @@ PHP_FUNCTION(timezone_name_get)

ZSTR_LEN(tmpstr) = snprintf(ZSTR_VAL(tmpstr), sizeof("+05:00"), "%c%02d:%02d",
utc_offset < 0 ? '-' : '+',
abs(utc_offset / 3600),
abs(((utc_offset % 3600) / 60)));
php_date_llabs(utc_offset / 3600),
php_date_llabs(((utc_offset % 3600) / 60)));

RETURN_NEW_STR(tmpstr);
}
Expand Down

4 comments on commit c189845

@derickr
Copy link
Contributor

@derickr derickr commented on c189845 Nov 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted this, as it broke ext/date/tests/bug66985.phpt on 32-bit platforms.

@morrisonlevi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@derickr So what did this break?

Why are we defining certain things like this?

#ifdef PHP_WIN32
static __inline __int64 php_date_llabs( __int64 i ) { return i >= 0? i: -i; }
#elif defined(__GNUC__) && __GNUC__ < 3
static __inline __int64_t php_date_llabs( __int64_t i ) { return i >= 0 ? i : -i; }
#else
static inline long long php_date_llabs( long long i ) { return i >= 0 ? i : -i; }
#endif

Why don't we do this since int64_t is already defined by php_stdint.h?:

static inline int64_t php_date_llabs( int64_t i ) { return i >= 0 ? i : -i; }

I suspect all this re-detection of sizes of things is part of the issue. As far as I can tell the utc_offset is supposed to be an int64_t and php_date_llabs is designed to work with those. This change should have been more correct. So what broke?

@derickr
Copy link
Contributor

@derickr derickr commented on c189845 Dec 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Bug #66985 (Some timezones are no longer valid in PHP 5.5.10) [ext/date/tests/bug66985.phpt]
=====================================================================
[GIT: (tags/php-7.2.0RC6~3)]
derick@singlemalt:~/dev/php/php-src.git $ diff -u ext/date/tests/bug66985.exp ext/date/tests/bug66985.out
--- ext/date/tests/bug66985.exp	2017-12-01 11:48:33.455823208 +0000
+++ ext/date/tests/bug66985.out	2017-12-01 11:48:33.455823208 +0000
@@ -151,5 +151,5 @@
 DateTimeZone Object
 (
     [timezone_type] => 1
-    [timezone] => -02:30
+    [timezone] => -02:00
 )
\ No newline at end of file

derick@singlemalt:~/dev/php/php-src.git $ sapi/cli/php -v
PHP 7.2.0-dev (cli) (built: Dec  1 2017 11:47:51) ( NTS DEBUG )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.2.0-dev, Copyright (c) 1998-2017 Zend Technologies
[GIT: (tags/php-7.2.0RC6~3)]

From c189845

@morrisonlevi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Please sign in to comment.