[GitHub] [trafficserver] bneradt commented on a diff in pull request #9509: Reduce the size of the APIHooks, eliminating enum gap

["bneradt (via GitHub)" <gi...@apache.org>]

bneradt commented on code in PR #9509:
URL: https://github.com/apache/trafficserver/pull/9509#discussion_r1131484156


##########
include/ts/apidefs.h.in:
##########
@@ -481,7 +481,7 @@ typedef enum {
 
   // Putting the SSL hooks in the same enum space
   // So both sets of hooks can be set by the same Hook function
-  TS_SSL_FIRST_HOOK        = 201,
+  TS_SSL_FIRST_HOOK,

Review Comment:
   Maybe my intentions were misplaced, but I intentionally added this gap here to allow room for new `TS_HTTP_*` values without breaking compatibility:
   https://github.com/apache/trafficserver/pull/8066
   
   Note that any new `TS_HTTP_*` value requires a corresponding `TS_EVENT_HTTP_*` value at the same offset. But that's not the case for `TS_SSL_*` values. Therefore, any new `TS_HTTP_*` value added _must_ be added before the `TS_SSL_*` values, and therefore, without the gap, must break compatibility because all subsequent `TS_SSL` values would be shifted. This was not understood when someone added `TS_HTTP_REQUEST_BUFFER_READ_COMPLETE_HOOK` at the end of this enum after `TS_SSL_*`, which I'm sure was done to try to not break compatibility. I had to fix that in #8066.
   
   Maybe allowing for new `TS_HTTP_*` values without breaking compatibility isn't valuable enough to justify the gap though. Can you explain what the problem is with having the gap?



-- 
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