You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "JosiahWI (via GitHub)" <gi...@apache.org> on 2026/06/08 11:41:01 UTC
[PR] Move some directory helpers to `Directory` [trafficserver]
JosiahWI opened a new pull request, #13245:
URL: https://github.com/apache/trafficserver/pull/13245
This is a follow up on #11945 which @masaori335 requested. It moves a handful of helper functions (those that need to access members of `Directory`) to the `Directory` type, so that they become methods. Some of them were `inline`; I have kept them `inline` to preserve the intent, so those function definitions have been moved to the header file.
--
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
Re: [PR] Move some directory helpers to `Directory` [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on code in PR #13245:
URL: https://github.com/apache/trafficserver/pull/13245#discussion_r3375306705
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -357,7 +308,7 @@ dir_clean_bucket(Dir *b, int s, StripeSM *stripe)
#ifdef LOOP_CHECK_MODE
loop_count++;
if (loop_count > DIR_LOOP_THRESHOLD) {
- if (dir_bucket_loop_fix(b, s, vol->directory))
+ if (vol->directory.bucket_loop_fix(b, s))
return;
Review Comment:
I want to track this in a separate bugfix for traceability.
--
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
Re: [PR] Move some directory helpers to `Directory` [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on PR #13245:
URL: https://github.com/apache/trafficserver/pull/13245#issuecomment-4649138743
[approve ci autest 1]
--
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
Re: [PR] Move some directory helpers to `Directory` [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13245:
URL: https://github.com/apache/trafficserver/pull/13245#discussion_r3375270408
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -260,7 +260,7 @@ Directory::bucket_length(Dir *b, int s)
int i = 0;
Dir *seg = this->get_segment(s);
#ifdef LOOP_CHECK_MODE
- if (dir_bucket_loop_fix(b, s, this))
+ if (this->bucket_loop_fix(b, s, this))
return 1;
#endif
Review Comment:
`Directory::bucket_loop_fix` now takes only `(Dir*, int)`, but this call still passes a third argument (`this`). This will fail to compile when `LOOP_CHECK_MODE` is enabled.
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -357,7 +308,7 @@ dir_clean_bucket(Dir *b, int s, StripeSM *stripe)
#ifdef LOOP_CHECK_MODE
loop_count++;
if (loop_count > DIR_LOOP_THRESHOLD) {
- if (dir_bucket_loop_fix(b, s, vol->directory))
+ if (vol->directory.bucket_loop_fix(b, s))
return;
Review Comment:
Inside `dir_clean_bucket`, the updated loop-fix call references `vol->directory`, but there is no `vol` variable in this scope (and the function already has `StripeSM *stripe`). This will not compile when `LOOP_CHECK_MODE` is enabled.
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -733,7 +684,7 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del)
#ifdef LOOP_CHECK_MODE
loop_count++;
if (loop_count > DIR_LOOP_THRESHOLD) {
- if (dir_bucket_loop_fix(dir_bucket(b, seg), s, this))
+ if (this->bucket_loop_fix(dir_bucket(b, seg), s, this))
return 0;
Review Comment:
`Directory::bucket_loop_fix` now takes only `(Dir*, int)`, but this call in `Directory::remove` still passes a third argument (`this`). This will fail to compile when `LOOP_CHECK_MODE` is enabled.
--
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
Re: [PR] Move some directory helpers to `Directory` [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI merged PR #13245:
URL: https://github.com/apache/trafficserver/pull/13245
--
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
Re: [PR] Move some directory helpers to `Directory` [trafficserver]
Posted by "JosiahWI (via GitHub)" <gi...@apache.org>.
JosiahWI commented on code in PR #13245:
URL: https://github.com/apache/trafficserver/pull/13245#discussion_r3375243020
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -1068,7 +1019,7 @@ CacheSync::mainEvent(int event, Event * /* e ATS_UNUSED */)
/* Don't sync the directory to disk if its not dirty. Syncing the
clean directory to disk is also the cause of INKqa07151. Increasing
the serial number causes the cache to recover more data than necessary.
- The dirty bit it set in dir_insert, overwrite and dir_delete_entry
+ The dirty bit it set in dir_insert, overwrite and Directory::delete_entry
Review Comment:
I applied the fix, but this is unrelated to the PR, so I'm downvoting it.
--
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
Re: [PR] Move some directory helpers to `Directory` [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13245:
URL: https://github.com/apache/trafficserver/pull/13245#discussion_r3374752397
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -1068,7 +1019,7 @@ CacheSync::mainEvent(int event, Event * /* e ATS_UNUSED */)
/* Don't sync the directory to disk if its not dirty. Syncing the
clean directory to disk is also the cause of INKqa07151. Increasing
the serial number causes the cache to recover more data than necessary.
- The dirty bit it set in dir_insert, overwrite and dir_delete_entry
+ The dirty bit it set in dir_insert, overwrite and Directory::delete_entry
Review Comment:
Minor grammar issue in this updated comment: "The dirty bit it set" should be "The dirty bit is set".
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -260,7 +260,7 @@ Directory::bucket_length(Dir *b, int s)
int i = 0;
Dir *seg = this->get_segment(s);
#ifdef LOOP_CHECK_MODE
- if (dir_bucket_loop_fix(b, s, this))
+ if (this->bucket_loop_fix(b, s, this))
return 1;
Review Comment:
`Directory::bucket_loop_fix` is now a member function with signature `(Dir *start_dir, int s)`, but this call still passes the old third `Directory*` argument. This will not compile when `LOOP_CHECK_MODE` is enabled.
##########
src/iocore/cache/CacheDir.cc:
##########
@@ -733,7 +684,7 @@ Directory::remove(const CacheKey *key, StripeSM *stripe, Dir *del)
#ifdef LOOP_CHECK_MODE
loop_count++;
if (loop_count > DIR_LOOP_THRESHOLD) {
- if (dir_bucket_loop_fix(dir_bucket(b, seg), s, this))
+ if (this->bucket_loop_fix(dir_bucket(b, seg), s, this))
return 0;
Review Comment:
`Directory::bucket_loop_fix` no longer takes a `Directory*` parameter. This call still passes a third argument, which will break compilation when `LOOP_CHECK_MODE` is enabled.
--
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