You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/03/22 20:00:03 UTC
[GitHub] [trafficserver] ywkaras opened a new pull request #7614: Change ROUNDUP from function-like macro to function template.
ywkaras opened a new pull request #7614:
URL: https://github.com/apache/trafficserver/pull/7614
Making it safe for argument expressions with side effects.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ezelkow1 removed a comment on pull request #7614: Change ROUNDUP from function-like macro to function template.
Posted by GitBox <gi...@apache.org>.
ezelkow1 removed a comment on pull request #7614:
URL: https://github.com/apache/trafficserver/pull/7614#issuecomment-804355127
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] alficles commented on a change in pull request #7614: Change ROUNDUP from function-like macro to function template.
Posted by GitBox <gi...@apache.org>.
alficles commented on a change in pull request #7614:
URL: https://github.com/apache/trafficserver/pull/7614#discussion_r599079228
##########
File path: include/tscore/ink_defs.h
##########
@@ -126,6 +122,21 @@ int ink_sys_name_release(char *name, int namelen, char *release, int releaselen)
int ink_number_of_processors();
int ink_login_name_max();
+#ifdef __cplusplus
+// Round up a value to be a multiple of m if necessary.
+//
+template <typename ArithmeticV, typename ArithmeticM>
+constexpr ArithmeticV
+ROUNDUP(ArithmeticV value, ArithmeticM m)
+{
+ ArithmeticV remainder = value % m;
+ if (remainder) {
+ value += m - remainder;
+ }
+ return value;
+}
Review comment:
This only provides it when it's included from a C++ file. It's a little odd to remove the C version when converting to C++. This version is unambiguously better, though, since it will refuse to compile with non-integer types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ezelkow1 commented on pull request #7614: Change ROUNDUP from function-like macro to function template.
Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on pull request #7614:
URL: https://github.com/apache/trafficserver/pull/7614#issuecomment-804355127
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #7614: Change ROUNDUP from function-like macro to function template.
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7614:
URL: https://github.com/apache/trafficserver/pull/7614#issuecomment-804398079
@alficles Can you please review this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras merged pull request #7614: Change ROUNDUP from function-like macro to function template.
Posted by GitBox <gi...@apache.org>.
ywkaras merged pull request #7614:
URL: https://github.com/apache/trafficserver/pull/7614
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7614: Change ROUNDUP from function-like macro to function template.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7614:
URL: https://github.com/apache/trafficserver/pull/7614#discussion_r603303041
##########
File path: include/tscore/ink_defs.h
##########
@@ -126,6 +122,21 @@ int ink_sys_name_release(char *name, int namelen, char *release, int releaselen)
int ink_number_of_processors();
int ink_login_name_max();
+#ifdef __cplusplus
+// Round up a value to be a multiple of m if necessary.
+//
+template <typename ArithmeticV, typename ArithmeticM>
+constexpr ArithmeticV
+ROUNDUP(ArithmeticV value, ArithmeticM m)
+{
+ ArithmeticV remainder = value % m;
+ if (remainder) {
+ value += m - remainder;
+ }
+ return value;
+}
Review comment:
ink_defs.h is included by .c files, but they don't use ROUNDUP.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org