ext_time_quota_acl: convert to c++ #1847

kinkie · 2024-06-24T21:37:14Z

Make use of dynamically-allocated strings
instead of static buffers, and convert debug output
to use Squid's Debug API

Inspired by addressing Coverity CID 1461163
"Invalid type in argument to printf format specifier"

kinkie · 2024-06-24T21:46:49Z

This has been runtime-tested:

./btbuild/src/acl/external/time_quota/ext_time_quota_acl -d -n -b /tmp/db ../timequota.def
1719265571 INFO| Starting ./btbuild/src/acl/external/time_quota/ext_time_quota_acl
1719265571 INFO| opening time quota database "/tmp/db".
1719265571 DEBUG| Database contains 17 entries.
1719265571 INFO| reading config file "../timequota.def".
1719265571 DEBUG| read config line 2: "john 8h / 1d"
1719265571 DEBUG| read time quota for user "john": 28800s / 86400s starting 1719187200
1719265571 DEBUG| writeTime("john-period-start", 1719187200)
1719265571 DEBUG| writeTime("john-period-length-configured", 86400)
1719265571 DEBUG| writeTime("john-time-budget-configured", 28800)
1719265571 DEBUG| readTime("john-time-budget-configured")=28800
1719265571 DEBUG| writeTime("john-time-budget-left", 28800)
1719265571 DEBUG| read config line 3: "littlejoe 1h / 1d"
1719265571 DEBUG| read time quota for user "littlejoe": 3600s / 86400s starting 1719187200
1719265571 DEBUG| writeTime("littlejoe-period-start", 1719187200)
1719265571 DEBUG| writeTime("littlejoe-period-length-configured", 86400)
1719265571 DEBUG| writeTime("littlejoe-time-budget-configured", 3600)
1719265571 DEBUG| readTime("littlejoe-time-budget-configured")=3600
1719265571 DEBUG| writeTime("littlejoe-time-budget-left", 3600)
1719265571 DEBUG| read config line 4: "babymary 30m / 1w"
1719265571 DEBUG| read time quota for user "babymary": 1800s / 604800s starting 1719187200
1719265571 DEBUG| writeTime("babymary-period-start", 1719187200)
1719265571 DEBUG| writeTime("babymary-period-length-configured", 604800)
1719265571 DEBUG| writeTime("babymary-time-budget-configured", 1800)
1719265571 DEBUG| readTime("babymary-time-budget-configured")=1800
1719265571 DEBUG| writeTime("babymary-time-budget-left", 1800)
1719265571 DEBUG| read config line 5: "short 1m / 1h"
1719265571 DEBUG| read time quota for user "short": 60s / 3600s starting 1719262800
1719265571 DEBUG| writeTime("short-period-start", 1719262800)
1719265571 DEBUG| writeTime("short-period-length-configured", 3600)
1719265571 DEBUG| writeTime("short-time-budget-configured", 60)
1719265571 DEBUG| readTime("short-time-budget-configured")=60
1719265571 DEBUG| writeTime("short-time-budget-left", 60)
1719265571 INFO| Waiting for requests...
short
1719265573 DEBUG| processActivity("short")
1719265573 DEBUG| readTime("short-period-start")=1719262800
1719265573 DEBUG| readTime("short-period-length-configured")=3600
1719265573 DEBUG| readTime("short-last-activity")=1719265499
1719265573 DEBUG| writeTime("short-last-activity", 1719265573)
1719265573 DEBUG| Time budget reduced by 74 for user "short".
1719265573 DEBUG| readTime("short-time-budget-left")=60
1719265573 DEBUG| writeTime("short-time-budget-left", -14)
1719265573 DEBUG| readTime("short-time-budget-left")=-14
1719265573 DEBUG| ERR message="Remaining quota for 'short' is -14 seconds."
ERR Time budget exceeded.
1719265577 INFO| Ending ./btbuild/src/acl/external/time_quota/ext_time_quota_acl

rousskov

This is a partial review in hope to help you may steady progress with this helper conversion.

src/acl/external/time_quota/ext_time_quota_acl.cc

src/acl/external/time_quota/ext_time_quota_acl.8

Inspired by Coverity CID 1461163, "Invalid type in argument to printf format specifier" Rely on c++ ostreams for proper handling of integer types

…tead of printf

Co-authored-by: Alex Rousskov <[email protected]>

Co-authored-by: Amos Jeffries <[email protected]>

src/acl/external/time_quota/ext_time_quota_acl.cc

yadij

Thank you. Looks good enough to me.
Just waiting on @rousskov discussions now.

rousskov · 2024-08-12T13:44:45Z

Just waiting on @rousskov discussions now.

FWIW, I have nothing to add to my last outstanding change request. That code changed a bit, but the problem remains. The ball there is on @kinkie side.

kinkie · 2024-08-12T16:48:10Z

Just waiting on @rousskov discussions now.

FWIW, I have nothing to add to my last outstanding change request. That code changed a bit, but the problem remains. The ball there is on @kinkie side.

For some reason this was not appearing in my log.
I'm adopting the "improved" version, minus the Debug::Extra bits which I find functionally bad due to not allowing easy grep(1)-ing.

Also added a trailing `<<` to ease source code comprehension, search, and adaptation.

rousskov

Still no objections to this PR going in...

src/acl/external/time_quota/ext_time_quota_acl.cc

Make use of dynamically-allocated strings instead of static buffers, and convert debug output to use Squid's Debug API Inspired by addressing Coverity CID 1461163 "Invalid type in argument to printf format specifier"

squid-anubis added the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Jun 24, 2024

kinkie changed the title ~~ext_time_quota_acl: convert to c++~~ ext_time_quota_acl: convert to c++ streams Jun 24, 2024

squid-anubis removed the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Jun 24, 2024

rousskov requested changes Jun 24, 2024

View reviewed changes

rousskov added the S-waiting-for-author author action is expected (and usually required) label Jun 24, 2024

yadij requested changes Jun 25, 2024

View reviewed changes

kinkie added 14 commits June 30, 2024 21:22

ext_time_quota_acl: convert logging to c++

8398381

Inspired by Coverity CID 1461163, "Invalid type in argument to printf format specifier" Rely on c++ ostreams for proper handling of integer types

display program name, not file name

94fb5a8

c++-ify KeyString

4b11887

add option to not clear the database at startup

fa1caad

refactor clear-if-first, loadTime

c116021

rely less on #defines

2d7e4e9

define -> const int

e598395

source maintenance

b8042f9

refactor constant names, count db entries on startup, use streams ins…

7eb1add

…tead of printf

no need for unistd

66c7b7c

clarify comment

c5c9699

Fix inverted clearDb flag

7e3c6e6

do not include iostream

a248f42

use size_t

fa7e4ac

kinkie force-pushed the coverity-cid-1461163 branch from 86f4410 to fa7e4ac Compare June 30, 2024 21:25

kinkie and others added 8 commits July 1, 2024 08:29

move from std::string to SBuf

f6eabe3

no need to specify dependencies explicitly

9161c0d

Source maintenance

41fca88

use auto

ceae1d4

Co-authored-by: Alex Rousskov <[email protected]>

use HLP_MSG

fe59146

Co-authored-by: Amos Jeffries <[email protected]>

do not log messages to squid

d17e976

use debugs() for output

bdfe687

update manual

d7aed3b

kinkie removed the S-waiting-for-author author action is expected (and usually required) label Aug 12, 2024

yadij requested changes Aug 12, 2024

View reviewed changes

src/acl/external/time_quota/ext_time_quota_acl.cc Outdated Show resolved Hide resolved

Change debug section

f2fb1f5

yadij previously approved these changes Aug 12, 2024

View reviewed changes

rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Aug 12, 2024

change error message on key size mismatch

6a6f126

kinkie dismissed yadij’s stale review via 6a6f126 August 12, 2024 16:48

kinkie added S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box and removed S-waiting-for-author author action is expected (and usually required) labels Aug 12, 2024

kinkie requested a review from rousskov August 12, 2024 17:13

fixup: Classify a level-1 error as such

f2e131a

Also added a trailing `<<` to ease source code comprehension, search, and adaptation.

rousskov reviewed Aug 12, 2024

View reviewed changes

src/acl/external/time_quota/ext_time_quota_acl.cc Outdated Show resolved Hide resolved

rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Aug 12, 2024

Source maintenance

f86004d

kinkie added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-author author action is expected (and usually required) labels Aug 13, 2024

squid-anubis closed this Aug 13, 2024

kinkie mentioned this pull request Nov 8, 2024

ext_time_quota_acl: restore debug level feature and argument #1936

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ext_time_quota_acl: convert to c++ #1847

ext_time_quota_acl: convert to c++ #1847

kinkie commented Jun 24, 2024 •

edited

Loading

kinkie commented Jun 24, 2024 •

edited

Loading

rousskov left a comment

yadij left a comment

rousskov commented Aug 12, 2024

kinkie commented Aug 12, 2024

rousskov left a comment

ext_time_quota_acl: convert to c++ #1847

ext_time_quota_acl: convert to c++ #1847

Conversation

kinkie commented Jun 24, 2024 • edited Loading

kinkie commented Jun 24, 2024 • edited Loading

rousskov left a comment

Choose a reason for hiding this comment

yadij left a comment

Choose a reason for hiding this comment

rousskov commented Aug 12, 2024

kinkie commented Aug 12, 2024

rousskov left a comment

Choose a reason for hiding this comment

kinkie commented Jun 24, 2024 •

edited

Loading

kinkie commented Jun 24, 2024 •

edited

Loading