You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "jasmine-nahrain (via GitHub)" <gi...@apache.org> on 2023/09/11 04:49:18 UTC

[GitHub] [trafficserver] jasmine-nahrain opened a new pull request, #10394: CID 1518583: Unchecked return value

jasmine-nahrain opened a new pull request, #10394:
URL: https://github.com/apache/trafficserver/pull/10394

   (no comment)


-- 
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] masaori335 commented on pull request #10394: CID 1518583: Unchecked return value

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #10394:
URL: https://github.com/apache/trafficserver/pull/10394#issuecomment-1713163413

   [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] bneradt commented on a diff in pull request #10394: CID 1518583: Unchecked return value

Posted by "bneradt (via GitHub)" <gi...@apache.org>.
bneradt commented on code in PR #10394:
URL: https://github.com/apache/trafficserver/pull/10394#discussion_r1321790511


##########
plugins/experimental/cache_fill/cache_fill.cc:
##########
@@ -72,7 +72,10 @@ cont_check_cacheable(TSHttpTxn txnp)
     return false;
   }
   int lookupStatus;
-  TSHttpTxnCacheLookupStatusGet(txnp, &lookupStatus);
+  if (TSHttpTxnCacheLookupStatusGet(txnp, &lookupStatus) == TS_ERROR) {
+    TSError("[cache_fill][%s] Couldn't get cache status of object", __FUNCTION__);

Review Comment:
   Better to refer to PLUGIN_NAME rather than the literal "cache_fill". See:
   https://github.com/apache/trafficserver/pull/10394/files#diff-896f422b7d09fb7725036af2885bbbca071b822167a1f8a99fbcb3ccd9d46f40R124
   
   Also, it's probably better to not include the `__FUNCTION__` in an error message. You already worded the message to be unique enough to identify where the Error is coming from, so we should be in good shape as far as that goes.



-- 
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] masaori335 merged pull request #10394: CID 1518583: Unchecked return value

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 merged PR #10394:
URL: https://github.com/apache/trafficserver/pull/10394


-- 
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] masaori335 commented on a diff in pull request #10394: CID 1518583: Unchecked return value

Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #10394:
URL: https://github.com/apache/trafficserver/pull/10394#discussion_r1320991641


##########
plugins/experimental/cache_fill/cache_fill.cc:
##########
@@ -72,7 +72,10 @@ cont_check_cacheable(TSHttpTxn txnp)
     return false;
   }
   int lookupStatus;
-  TSHttpTxnCacheLookupStatusGet(txnp, &lookupStatus);
+  if (TSHttpTxnCacheLookupStatusGet(txnp, &lookupStatus) == TS_ERROR) {
+    TSError("[esi][%s] Couldn't get cache status of object", __FUNCTION__);

Review Comment:
   `esi` is the name of the other plugin. Please replace it with `cache_fill`.



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