Fix using macro with regex in path parameter in fbtrace.conf

[TreeHunter9]

Currently we cannot use macro with regex in path parameter because PathUtils::fixupSeparators(value.begin()); happens too early, and it will fix \1 to /1 before we replace it with regex group, so we get unexpected path. On Windows, we just get an error, because PathUtils::fixupSeparators(value.begin()); will change all / to \, and it will cause expandPattern() to throw an exception.

[aafemt]

Some problems are hardly avoidable when same char is used for 2 different purposes, but lets try to minimize them.

If you are ready to review the initial design, I would suggest to use $(1) for subpattern substitutions.

[AlexPeshkoff]

On 9/5/24 17:52, Dimitry Sibiryakov wrote:

Some problems are hardly avoidable when same char is used for 2
different purposes, but lets try to minimize them.

If you are ready to review the initial design, I would suggest to use |$(1)| for subpattern substitutions.

Looks as a minimum interesting - but discussion is devel is needed for such change.

[TreeHunter9]

I'm afraid suggested solution is not complete. Imagine someone types (in windows): log_filename = $(root)\audit-logs\1.log Some problems are hardly avoidable when same char is used for 2 different purposes, but lets try to minimize them.

Oh, for some reason I thought only / was allowed as a separator in the path. Now I looked more into it and found out that we should use \\ for backslashes in path (as it says in fbtrace.conf), and here is code snippet from TraceCfgReader::expandPattern: https://github.com/FirebirdSQL/firebird/blob/72cc0880e25c8fbd8020d56d6e2700f1c7e493c1/src/utilities/ntrace/TraceConfiguration.cpp#L291-L297 So I guess the problem only remains on Linux because of PathUtils::fixupSeparators(value.begin()); will fixup \1 regex pattern. I will do another test tomorrow.

[TreeHunter9]

I'm afraid suggested solution is not complete. Imagine someone types (in windows): log_filename = $(root)\audit-logs\1.log

We should write path like this: $(root)audit-logs\\\1.log, $(root) already have \\ at the end, and audit-logs\\\1.log so we can translate it into audit-logs\employee.log. So I guess we don't need to change the semantics of the regexp. So we really only have one problem, where macro insertion fixes all regexp patterns (on Windows macro insertion uses incorrect backslashes for path) and this patch should fix that.

[AlexPeshkoff]

Macro in config design supposed to provide smart behavior when merging paths, i.e. one need not know that $(root) already has \ at the end. In linux having double dir separators one after another (something like /usr//bin instead /usr/bin) is not dangerous but I'm not sure for other OS-es. Anyway, resulting file name may get visible to user somewhere else, i.e. I prefer to have correctly merged paths, not relying on presence of // at the end. Also we always tried to have config format as OS independent as possible, i.e. one should be able to have $(root)\audit-logs\\1.log on linux or $(root)/audit-logs/\1.log on windows (may be as a result of file copy when changing OS).

[TreeHunter9]

Tested on these file paths:

Windows and Linux:
$(root)\1.log -> /path/to/root/employee.log
$(root)/\1.log -> /path/to/root/employee.log
$(root)\\\1.log -> /path/to/root/employee.log
$(root)\\1.log -> /path/to/root/1.log

Only Linux:
/$(root)/\1.log -> /path/to/root/employee.log
\\$(root)/\1.log -> /path/to/root/employee.log

[AlexPeshkoff]

I suggest you backport changes to previous versions

[dyemanov]

I will backport it.