On 4/11/2019 10:29 PM, Junio C Hamano wrote:
"Jeff Hostetler via GitGitGadget" <[email protected]> writes:
[...]
@@ -512,19 +454,28 @@ + */ +/* clang-format off */ ++ [TR2_SYSENV_CFG_PARAM] = { "GIT_TR2_CONFIG_PARAMS", ++ "trace2.configparams" }, ++ ++ [TR2_SYSENV_DST_DEBUG] = { "GIT_TR2_DST_DEBUG", ++ "trace2.destinationdebug" }, ++ ++ [TR2_SYSENV_NORMAL] = { "GIT_TR2", ++ "trace2.normaltarget" }, ++ [TR2_SYSENV_NORMAL_BRIEF] = { "GIT_TR2_BRIEF", ++ "trace2.normalbrief" }, ++ ++ [TR2_SYSENV_EVENT] = { "GIT_TR2_EVENT", ++ "trace2.eventtarget" }, ++ [TR2_SYSENV_EVENT_BRIEF] = { "GIT_TR2_EVENT_BRIEF", ++ "trace2.eventbrief" }, ++ [TR2_SYSENV_EVENT_NESTING] = { "GIT_TR2_EVENT_NESTING", ++ "trace2.eventnesting" }, ++ ++ [TR2_SYSENV_PERF] = { "GIT_TR2_PERF", ++ "trace2.perftarget" }, ++ [TR2_SYSENV_PERF_BRIEF] = { "GIT_TR2_PERF_BRIEF", ++ "trace2.perfbrief" },With use of designated initializers, the table got a lot cleaner to read. Is the above "format off" still needed (I am a bit curious how clang-format wants these entries to look like)?
clang-format suggests getting rid of the extra whitespace on the lines, so we lose all of the column alignment. Then it wants to line-wrap some but not all of the lines. So it is a bit of a mess to look at.
++ if (pid > 999999) ++ strbuf_addf(&tr2sid_buf, "W%06d", (int)(pid % 1000000)); ++ else ++ strbuf_addf(&tr2sid_buf, "P%06d", (int)pid);I do not think it matters too much, but this is kind-of curious. How would the users of the log utilize the distinction between W and P? Do they discard the ones with W when they care about the exact process that left the trace entries, or something? If it's not a plausibly useful use pattern (and I do not think it is), I wonder if we want to go with only W (i.e. truncated to the lower N digits) entries, if you are shooting for a fixed-width output from this function. If you want less chance of collisions, you obviously could use hexadecimal to gain back a few more bits. After all, if the application does care the PID, that could be in the log data itself (i.e. an "start" event can say "my pid is blah").
Right. Ævar suggested adding the full or wrapped PID so that the SID would be a fixed length. I stuck with decimal rather than hex because it's easier to match up a running command with '/usr/bin/ps' output, but that's no big deal either way. It might be simpler to just %08lx it and be done with it. Jeff
