Re: [PATCH v2 39/39] util/log: Support per-thread log files

[Richard Henderson]

On 4/14/22 08:35, Alex Bennée wrote:
> > +/**
> > + * valid_filename_template:
> > + *
> > + * Validate the filename template.  Require %d if per_thread, allow it
> > + * otherwise; require no other % within the template.
> > + * Return 0 if invalid, 1 if stderr, 2 if strdup, 3 if pid printf.
>
>  From a neatness point of view having an enum would make it easier to
> read in the switch down bellow...

Fair, and...

> > +        switch (valid_filename_template(filename, per_thread, errp)) {
> > +        case 0:
> > +            return false; /* error */
> > +        case 1:
> > +            break;  /* stderr */
> > +        case 2:
> > +            newname = g_strdup(filename);
> > +            break;
> > +        case 3:
> > +            newname = g_strdup_printf(filename, getpid());
> > +            break;
>
> default: g_assert_not_reached?

... using an enum with an extra enumerator for testing gives us

../src/util/log.c: In function ‘qemu_set_log_internal’:
../src/util/log.c:213:9: error: enumeration value ‘vft_new_case’ not handled in switch 
[-Werror=switch]
  213 |         switch (valid_filename_template(filename, per_thread, errp)) {
      |         ^~~~~~
cc1: all warnings being treated as errors

though that does require *not* having a default case.


r~