Return to Answer

well format it

Source Link

John

Having a bunch of global variables grouped by the virtue of spacing in the source file is very rarely a good idea. What you have here is a class in principle (a bunch of data plus methods), not using the class syntax for that is strange.
Since it is not a class ,there is no notion of what is private and public. As a consequence, you have several Log functions with drastically different parameters. This makes your code difficult to follow.
The code is not very good with separation of concerns. The code for formatting, opening files, working with file paths and writing is dumped into a single function with a strange gotogoto in it. If you split the formatting from the output, the code will become clearer and this gotogoto will not be needed.
Your double-check lock is incorrect. Since the variable is non-volatile, the compiler is free to reorder the calls or optimize them away. Being extern does not help.
It might be better to write exceptions to stderrstderr.
You should use ISO-8601 as your time format.It will save you a lot of trouble when other peroplepeople will read it or when you decide to write a script to datamine your logs, or upload them to a database and many other situations.
under a heavy load write/flush under a mutex might be a bottleneck

Having a bunch of global variables grouped by the virtue of spacing in the source file is very rarely a good idea. What you have here is a class in principle (a bunch of data plus methods), not using the class syntax for that is strange.
Since it is not a class ,there is no notion of what is private and public. As a consequence, you have several Log functions with drastically different parameters. This makes your code difficult to follow.
The code is not very good with separation of concerns. The code for formatting, opening files, working with file paths and writing is dumped into a single function with a strange goto in it. If you split the formatting from the output, the code will become clearer and this goto will not be needed.
Your double-check lock is incorrect. Since the variable is non-volatile, the compiler is free to reorder the calls or optimize them away. Being extern does not help.
It might be better to write exceptions to stderr
You should use ISO-8601 as your time format.It will save you a lot of trouble when other perople will read it or when you decide to write a script to datamine your logs, or upload them to a database and many other situations.
under a heavy load write/flush under a mutex might be a bottleneck

Having a bunch of global variables grouped by the virtue of spacing in the source file is very rarely a good idea. What you have here is a class in principle (a bunch of data plus methods), not using the class syntax for that is strange.
Since it is not a class ,there is no notion of what is private and public. As a consequence, you have several Log functions with drastically different parameters. This makes your code difficult to follow.
The code is not very good with separation of concerns. The code for formatting, opening files, working with file paths and writing is dumped into a single function with a strange goto in it. If you split the formatting from the output, the code will become clearer and this goto will not be needed.
Your double-check lock is incorrect. Since the variable is non-volatile, the compiler is free to reorder the calls or optimize them away. Being extern does not help.
It might be better to write exceptions to stderr.
You should use ISO-8601 as your time format.It will save you a lot of trouble when other people will read it or when you decide to write a script to datamine your logs, or upload them to a database and many other situations.
under a heavy load write/flush under a mutex might be a bottleneck

Source Link

answered Aug 22, 2021 at 21:33

vvotan

Having a bunch of global variables grouped by the virtue of spacing in the source file is very rarely a good idea. What you have here is a class in principle (a bunch of data plus methods), not using the class syntax for that is strange.
Since it is not a class ,there is no notion of what is private and public. As a consequence, you have several Log functions with drastically different parameters. This makes your code difficult to follow.
The code is not very good with separation of concerns. The code for formatting, opening files, working with file paths and writing is dumped into a single function with a strange goto in it. If you split the formatting from the output, the code will become clearer and this goto will not be needed.
Your double-check lock is incorrect. Since the variable is non-volatile, the compiler is free to reorder the calls or optimize them away. Being extern does not help.
It might be better to write exceptions to stderr
You should use ISO-8601 as your time format.It will save you a lot of trouble when other perople will read it or when you decide to write a script to datamine your logs, or upload them to a database and many other situations.
under a heavy load write/flush under a mutex might be a bottleneck