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/09/17 03:16:04 UTC
[GitHub] [trafficserver] maskit opened a new pull request #8331: Change the function singnature of safe_[get|set]sockopt
maskit opened a new pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331
The type of `optval` argument for [get|set]sockopt is `void*`, but not `char*`.
Some values don't fit into a `char`, and casting the pointer to `char*` causes compile error.
```
int getsockopt(int sockfd, int level, int optname,
void *optval, socklen_t *optlen);
int setsockopt(int sockfd, int level, int optname,
const void *optval, socklen_t optlen);
```
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on pull request #8331: Change the function signature of safe_[get|set]sockopt
Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331#issuecomment-1017064795
@bryancall I left this open for you because your comment was a change request, but I'm going to merge this with Brian's approval next week if there's no response.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] zwoop commented on pull request #8331: Change the function signature of safe_[get|set]sockopt
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331#issuecomment-1028461747
Cherry-picked to v9.2.x
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] github-actions[bot] commented on pull request #8331: Change the function signature of safe_[get|set]sockopt
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331#issuecomment-1017041272
This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #8331: Change the function signature of safe_[get|set]sockopt
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331#discussion_r717175181
##########
File path: include/tscore/ink_sock.h
##########
@@ -34,8 +34,8 @@
#include "tscore/ink_apidefs.h"
-int safe_setsockopt(int s, int level, int optname, char *optval, int optlevel);
-int safe_getsockopt(int s, int level, int optname, char *optval, int *optlevel);
+int safe_setsockopt(int s, int level, int optname, void *optval, int optlevel);
+int safe_getsockopt(int s, int level, int optname, void *optval, int *optlevel);
Review comment:
I kept my change minimal.
If I change `int *` to `socklen_t *` I have to deal with the error below at all places that call `safe_getsockopt`. I could just reinterpret_cast at all the places but I guess some of the places can declare variables as `unsigned int` and don't need any casts. I don't want to check and fix that.
```
/Users/mkitajo/src/github.com/trafficserver/iocore/eventsystem/P_UnixSocketManager.h:482:11: error: no matching function for call to 'safe_getsockopt'
r = safe_getsockopt(s, SOL_SOCKET, SO_SNDBUF, (char *)&bsz, &bszsz);
^~~~~~~~~~~~~~~
../../include/tscore/ink_sock.h:38:5: note: candidate function not viable: no known conversion from 'int *' to 'socklen_t *' (aka 'unsigned int *') for 5th argument
int safe_getsockopt(int s, int level, int optname, void *optval, socklen_t *optlevel);
^
```
I added `const` for `optval` for now.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall removed a comment on pull request #8331: Change the function signature of safe_[get|set]sockopt
Posted by GitBox <gi...@apache.org>.
randall removed a comment on pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331#issuecomment-943668866
[approve ci]
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #8331: Change the function signature of safe_[get|set]sockopt
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331#discussion_r717175181
##########
File path: include/tscore/ink_sock.h
##########
@@ -34,8 +34,8 @@
#include "tscore/ink_apidefs.h"
-int safe_setsockopt(int s, int level, int optname, char *optval, int optlevel);
-int safe_getsockopt(int s, int level, int optname, char *optval, int *optlevel);
+int safe_setsockopt(int s, int level, int optname, void *optval, int optlevel);
+int safe_getsockopt(int s, int level, int optname, void *optval, int *optlevel);
Review comment:
I kept my change minimal.
If I change `int *` to `socklen_t *` I have to deal with the error below at all places that call `safe_getsockopt`. I could just reinterpret_cast at all the places but I guess some of the places can declare variables as `unsigned int` and don't need any casts. I don't want to check and fix that.
```
/Users/mkitajo/src/github.com/trafficserver/iocore/eventsystem/P_UnixSocketManager.h:482:11: error: no matching function for call to 'safe_getsockopt'
r = safe_getsockopt(s, SOL_SOCKET, SO_SNDBUF, (char *)&bsz, &bszsz);
^~~~~~~~~~~~~~~
../../include/tscore/ink_sock.h:38:5: note: candidate function not viable: no known conversion from 'int *' to 'socklen_t *' (aka 'unsigned int *') for 5th argument
int safe_getsockopt(int s, int level, int optname, void *optval, socklen_t *optlevel);
^
```
I added `const` for `optval` for now.
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on a change in pull request #8331: Change the function signature of safe_[get|set]sockopt
Posted by GitBox <gi...@apache.org>.
bryancall commented on a change in pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331#discussion_r712572296
##########
File path: include/tscore/ink_sock.h
##########
@@ -34,8 +34,8 @@
#include "tscore/ink_apidefs.h"
-int safe_setsockopt(int s, int level, int optname, char *optval, int optlevel);
-int safe_getsockopt(int s, int level, int optname, char *optval, int *optlevel);
+int safe_setsockopt(int s, int level, int optname, void *optval, int optlevel);
+int safe_getsockopt(int s, int level, int optname, void *optval, int *optlevel);
Review comment:
If you are going to mirror the setsockopt API then shouldn't it be const void * and socklen_t for the length?
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit merged pull request #8331: Change the function signature of safe_[get|set]sockopt
Posted by GitBox <gi...@apache.org>.
maskit merged pull request #8331:
URL: https://github.com/apache/trafficserver/pull/8331
--
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.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org