Attention is currently required from: Felix Singer, Alan Green, Angel Pons, Michael Niewöhner.

Patch set 16:Code-Review +1

View Change

5 comments:

• Patchset:

  • Patch Set #16:

    Alan, would the new syntax suit your needs?

• File ft2232_spi.c:

  • Patch Set #16, Line 483: msg_perr

    As the message says "warning", use msg_pwarn()?

  • Patch Set #16, Line 484: "in the future. Use `gpiolX=C` instead.\n");

    We try to keep output within 80 columns. Could add a \n after the
    first sentence for instance.

  • Patch Set #16, Line 513: arg_gpiol[3] = extract_programmer_param("gpiol3");

    This looks a little odd. How about moving the call into the loop?
    We could still use an array for the string literals, e.g.

        const char *const gpiol_params[] = { "gpiol0", "gpiol1", ...
        char *const arg = extract_programmer_param(gpiol_params[pin]);

    Alternatively, a single buffer could be used like this:

        char gpiol_param[7];
        snprintf(gpiol_param, sizeof(gpiol_param), "gpiol%d", pin);
        char *const arg = extract_programmer_param(gpiol_param);

    Also just noticed: In the current form, any premature `return` inside
    the loop would leak some of these arg_gpiol[] pointers.

  • Patch Set #16, Line 538: bool format_error = 0;

    Just to mention it, don't know how reasonable it would look: For error
    paths, using `goto` is common. The end of the loop body could look like
    this:

                        free(arg);
                        continue;
        format_error:
                        msg_perr("Error: ...");
                        free(arg);
                        return -2;
                }

    It would save us an indentation level.

To view, visit change 57810. To unsubscribe, or for help writing mail filters, visit settings.