On 08/25/2016 08:18 AM, Eduard Bagdasaryan wrote: > 2016-08-24 18:20 GMT+03:00 Amos Jeffries <squ...@treenet.co.nz>: > >> in src/LogTags.cc: >> * instead of adding new enum entry please extend LogTags with a new bool >> flag and the c_str() to append the "IGNORED" when that flag is true.
> Added a new LogTags::ignored for this. The naming of things in logging code is exceptionally poor so please do not feel upset that you got this wrong. The "_IGNORED" flag Amos is talking about should go into LogTags::Errors. I recommend renaming and re-documenting that subclass: /// Things that may happen to a transaction while it is being /// processed according to its LOG_* category. Logged as _SUFFIX(es). /// Unlike LOG_* categories, these flags may not be mutually exclusive. class Flags { ... bool ignored; ///< _IGNORED: the response was not used for anything bool timedout; ///< _TIMEDOUT: terminated due to a lifetime or I/O timeout bool aborted; ///< _ABORTED: other abnormal termination (e.g., I/O error) } flags; This does not fix all the problems, but reduces the confusion considerably IMHO. > I wonder whether the > LogTags assignment operator should copy this member too. Which assignment operator? * The compiler-generated assignment operator will copy all members automatically, of course. * The explicitly-defined LogTags_ot conversion assignment operator is already buggy or dangerous. It should be removed. Whether it was meant to clear the flags, I do not know, unfortunately. I recommend leaving it as is because fixing it is out of scope and may not be trivial. Adding an "XXX: this operator does not reset flags" and "TODO: either replace with a category-only setter or remove" comments would be appropriate. HTH, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev