[v3,0/1] bugreport: include +i in outfile suffix as needed

Message

Jacob Stopak Oct. 16, 2023, 9:40 p.m. UTC

Similar functionality to the previous patch version but with the
following changes:

  * Remove the default_option_suffix variable and clean up the way the
    option_suffix value is set.

  * Refactor code for building report path and diagnostics zip file path
    into the new function build_path(...), which builds a fresh path
    from scratch each time it's called. Although it does take quite a few
    arguments, it reduces duplicated code and simplifies the code by
    removing the need for splicing or inserting the incrementable suffix.

  * Allow the increment value 'i' to grow as needed instead of stopping
    at 9.

  * Replace xopen() with open() so that the program doesn't die with an
    error if the file already exists.

  * Replace the while loop with a fallback loop to prevent TOCTOU.

  * Clean up commit message verbiage.

Some additional comments:

> Perhaps we should refine the error message we give in this case and
> we are done, then?

I had thought about this but due to the variable timed behavior I had
trouble coming up with a message that would convey clearly what's going
on and what the user should do. Here were some things that popped into
my head but they all sounded a bit silly to me:

"A bugreport with this name already exists, try again shortly"
or
"File exists: wait 1 minute and try again or use a custom suffix with -s"
or
"File exists: try again in 1 to 60 seconds :-P"

I think the reason they sound silly to me is that the user is only being
made aware of this info because the program happens to be operating this
way - instead of the program working in a smoother way where this type of
operational info wouldn't need to be conveyed.

> What downside do you see in using 2023-10-15-1008+10 after you tried
> 9 of them?  The code to limit the upper bound smells like a wasted
> effort that helps nobody in practice because it is "unlikely".

> And limiting the upper bound also means you now have to have extra
> code to deal with "we ran out---error out and help the user how to
> recover" anyway.

I completely agree with this and made these updates.

> Notice that you said "in a single minute" without "calendar" and the
> sentence is perfectly understandable?

I googled "calendar minute" and the only thing that came up was some
Java documentation... So I guess I just made it up... :'D

Jacob Stopak (1):
  bugreport: include +i in outfile suffix as needed

 builtin/bugreport.c | 83 +++++++++++++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 26 deletions(-)


base-commit: 493f4622739e9b64f24b465b21aa85870dd9dc09