Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ext_time_quota_acl: convert to c++ #1847

Closed
wants to merge 65 commits into from
Closed

ext_time_quota_acl: convert to c++ #1847

Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
8398381
ext_time_quota_acl: convert logging to c++
kinkie Jun 9, 2024
94fb5a8
display program name, not file name
kinkie Jun 23, 2024
4b11887
c++-ify KeyString
kinkie Jun 23, 2024
fa1caad
add option to not clear the database at startup
kinkie Jun 23, 2024
c116021
refactor clear-if-first, loadTime
kinkie Jun 24, 2024
2d7e4e9
rely less on #defines
kinkie Jun 24, 2024
e598395
define -> const int
kinkie Jun 24, 2024
b8042f9
source maintenance
kinkie Jun 24, 2024
7eb1add
refactor constant names, count db entries on startup, use streams ins…
kinkie Jun 24, 2024
66c7b7c
no need for unistd
kinkie Jun 24, 2024
c5c9699
clarify comment
kinkie Jun 24, 2024
7e3c6e6
Fix inverted clearDb flag
kinkie Jun 24, 2024
a248f42
do not include iostream
kinkie Jun 30, 2024
fa7e4ac
use size_t
kinkie Jun 30, 2024
f6eabe3
move from std::string to SBuf
kinkie Jul 1, 2024
9161c0d
no need to specify dependencies explicitly
kinkie Jul 1, 2024
41fca88
Source maintenance
kinkie Jul 1, 2024
ceae1d4
use auto
kinkie Jul 1, 2024
fe59146
use HLP_MSG
kinkie Jul 1, 2024
d17e976
do not log messages to squid
kinkie Jul 1, 2024
bdfe687
use debugs() for output
kinkie Jul 1, 2024
d7aed3b
update manual
kinkie Jul 1, 2024
0a2a1e1
disable debugging level selection
kinkie Jul 1, 2024
2e55189
remove no-clear-db option
kinkie Jul 1, 2024
af88dd7
Fix manual
kinkie Jul 1, 2024
f7375d7
Fix manual
kinkie Jul 1, 2024
733da8d
Source maintenance
kinkie Jul 1, 2024
111dff9
Fix manual formatting
kinkie Jul 1, 2024
354f70c
Clarify comment in src/Makefile.am
kinkie Jul 18, 2024
d60e6b8
TQ_BUFFERSIZE is static
kinkie Jul 18, 2024
3abbf32
Attempt to restore -l logfile
kinkie Jul 20, 2024
e2a7bdb
Enable logging to file
kinkie Jul 21, 2024
9154fb1
rename DEBUG_SECTION
kinkie Jul 21, 2024
7a80caf
Remove useless header
kinkie Jul 21, 2024
2f9404a
Remove useless header
kinkie Jul 21, 2024
44ba10b
reorder debug constant
kinkie Jul 21, 2024
08c842a
Revert changes to man page
kinkie Jul 21, 2024
51911b4
do not use variable for dbopts
kinkie Jul 21, 2024
d40eb9d
Use ToSBuf
kinkie Jul 21, 2024
0e22830
no need to null-terminate TDB_DATA
kinkie Jul 21, 2024
6f17335
Fix debug, restore help text
kinkie Jul 21, 2024
0b02366
Source format
kinkie Jul 21, 2024
243601a
use const TDB_DATA
kinkie Jul 21, 2024
f7cbf32
convert output to stderr to debugs
kinkie Jul 21, 2024
5850a98
Use ToSBuf instead of SBufStream
kinkie Jul 21, 2024
a57a3ec
Use ToSBuf
kinkie Jul 21, 2024
946a4f0
clarify debug message
kinkie Jul 22, 2024
592e959
Mark errors as such in log
kinkie Jul 22, 2024
f756548
source maintenance
kinkie Jul 22, 2024
5ecc131
fix typo in constant
kinkie Jul 22, 2024
1552a68
move argument parsing back where it was
kinkie Jul 23, 2024
0b06269
Add ChangeLog entry
kinkie Jul 26, 2024
8d0a12d
Revert "Add ChangeLog entry"
kinkie Jul 26, 2024
60223cd
Merge remote-tracking branch 'upstream/master' into coverity-cid-1461163
kinkie Aug 8, 2024
024a8c2
sbuf/Stream.h includes sbuf/SBuf.h
kinkie Aug 8, 2024
78d2448
format polishj
kinkie Aug 8, 2024
9bb3ed0
Revert changes to debug
kinkie Aug 9, 2024
aba454d
reorder LDADD
kinkie Aug 9, 2024
08cd067
Revert "reorder LDADD"
kinkie Aug 10, 2024
7a032a2
Change debug section
kinkie Aug 11, 2024
d1bfd78
Do not use variable for debug
kinkie Aug 11, 2024
f2fb1f5
Change debug section
kinkie Aug 12, 2024
6a6f126
change error message on key size mismatch
kinkie Aug 12, 2024
f2e131a
fixup: Classify a level-1 error as such
rousskov Aug 12, 2024
f86004d
Source maintenance
kinkie Aug 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ LOADABLE_MODULES_SOURCES = \
LoadableModules.cc \
LoadableModules.h

SUBDIRS = mem time debug base anyp helper dns html ftp parser comm error eui acl format clients sbuf servers fs repl store DiskIO proxyp
# order is important. If anything in a subdir requires a convenience library
# from another subdir, then it must appear later in this list
kinkie marked this conversation as resolved.
Show resolved Hide resolved
SUBDIRS = mem time debug base anyp helper dns html ftp parser comm error eui sbuf acl format clients servers fs repl store DiskIO proxyp
kinkie marked this conversation as resolved.
Show resolved Hide resolved

if ENABLE_AUTH
SUBDIRS += auth
Expand Down
8 changes: 8 additions & 0 deletions src/acl/external/time_quota/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,15 @@ DEFS += -DDEFAULT_QUOTA_DB=\"$(localstatedir)/ext_time_quota.db\"

ext_time_quota_acl_SOURCES = \
ext_time_quota_acl.cc

ext_time_quota_acl_LDADD = \
$(top_builddir)/src/sbuf/libsbuf.la \
$(top_builddir)/src/debug/libdebug.la \
$(top_builddir)/src/error/liberror.la \
$(top_builddir)/src/comm/libminimal.la \
$(top_builddir)/src/mem/libminimal.la \
$(top_builddir)/src/base/libbase.la \
$(top_builddir)/src/time/libtime.la \
Comment on lines +19 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dependencies can be slightly tricky to work out and we have a lot of technical debt in the existing lists.

For new additions; please use the SUBDIRS list in src/Makefile.am as the authoritative dependency sequence. LDADD should then list the needful objects in reverse order.

Suggested change
$(top_builddir)/src/sbuf/libsbuf.la \
$(top_builddir)/src/debug/libdebug.la \
$(top_builddir)/src/error/liberror.la \
$(top_builddir)/src/comm/libminimal.la \
$(top_builddir)/src/mem/libminimal.la \
$(top_builddir)/src/base/libbase.la \
$(top_builddir)/src/time/libtime.la \
$(top_builddir)/src/sbuf/libsbuf.la \
$(top_builddir)/src/error/liberror.la \
$(top_builddir)/src/comm/libminimal.la \
$(top_builddir)/src/base/libbase.la \
$(top_builddir)/src/debug/libdebug.la \
$(top_builddir)/src/time/libtime.la \
$(top_builddir)/src/mem/libminimal.la \

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For new additions; please use the SUBDIRS list in src/Makefile.am as the authoritative dependency sequence. LDADD should then list the needful objects in reverse order.

Let's not declare SUBDIRS order authoritative for LDADD: SUBDIRS determine build dependency order among subdirectories. LDADD determines linking order. One does not have to be the reversed version of the other (and LDADD order is often more strict/important/difficult to get right). In cases where one subdirectory contains multiple compatible (i.e. not mutually exclusive) libraries, SUBDIRS order cannot determine LDADD order at all!

For consistency sake, let's start new (or substantially new) LDADD lists with (reversed) SUBDIRS order and then adjust LDADD (and sometimes SUBDIRS!) as needed, without declaring SUBDIRS order as authoritative.

P.S. This comment is not endorsing or objecting to specific order suggested on this change request thread.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me whether this change should go in or not. The code as in this PR builds. Why should I change it? Why should I not change it? Why should fixing LDADD or SUDIRS beyond what is needed to build this PR be in scope for this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should I change it?

You should change it to quickly address this change request and/or if you believe the suggested order is better (for any reason).

Why should I not change it?

You should not change it if you are worried that it will set a bad precedent (despite my objections to establishing the corresponding rule) and we will have to modify other LDADD lines whenever we modify one of them and/or we will have to modify LDADD lines whenever we reorder SUBDIRS.

Why should fixing LDADD or SUDIRS beyond what is needed to build this PR be in scope for this PR?

It is clearly out of this PR scope. However, we do make exceptions for trivial out-of-scope changes of already modified code lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I see the logic in the change. I agree to not setting additional rules - we have too many already. It already works but won't hurt so ahoy!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It already works but won't hurt so ahoy!

It did hurt AFAICT. To avoid misunderstanding, I am still OK with any order you two (and CI) can agree on here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted to previous order, which works

$(COMPAT_LIB) \
$(LIBTDB_LIBS)

Expand Down
10 changes: 2 additions & 8 deletions src/acl/external/time_quota/ext_time_quota_acl.8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Version 1.0
.
.SH SYNOPSIS
.if !'po4a'hide' .B ext_time_quota_acl
.if !'po4a'hide' .B "[\-b database] [\-l logfile] [\-d] [\-p pauselen] [\-h] configfile
.if !'po4a'hide' .B "[\-b database] [\-d] [\-p pauselen] [\-h] configfile
.
.SH DESCRIPTION
.B ext_time_quota_acl
Expand Down Expand Up @@ -35,14 +35,8 @@ Pauses shorter than this value will be counted against the quota, longer ones ig
Default is 300 seconds (5 minutes).
.
.if !'po4a'hide' .TP
.if !'po4a'hide' .B "\-l logfile"
.B Filename
where all logging and debugging information will be written. If none is given,
then stderr will be used and the logging will go to Squids main cache.log.
kinkie marked this conversation as resolved.
Show resolved Hide resolved
.
.if !'po4a'hide' .TP
.if !'po4a'hide' .B "\-d"
Enables debug logging in the logfile.
Enables debug logging to stderr.
yadij marked this conversation as resolved.
Show resolved Hide resolved
.
.if !'po4a'hide' .TP
.if !'po4a'hide' .B "\-h"
Expand Down
Loading