Re: [PATCH v4 5/9] wispr: Refactor 'wispr_portal_web_result' to leverage 'GError'.
Denis Kenzior <[email protected]> Tue, 25 Mar 2025 11:53:29 -0500
| Newsgroups | dev.linux.lists.connman |
|---|---|
| Message-ID | <[email protected]> |
Hi Grant,
>>
>> But now you have a simple return at the end of the function. Which by the way is discouraged and should have been picked up by the compiler or static analysis.
>
> I tried to keep the original block of logic as-is.
>
> Personally, I’m a fan of single points of exit—so much easier to audit. But, happy to keep
Kernel often prefers the 'exit-early' approach, but this is situational.
whatever style is preferred. It sounds like the preference is to replace the
prior ‘goto done’ with ‘return’, correct?
Rule of thumb is not to have return at the end of a void function. Static
analysis will for sure flag that and I'll kick off a coverity run once I get the
rest of your patches merged.
The question really is: Is the now-empty-done label hiding a bug?
>
>>> + g_web_parser_end_data(wp_context->wispr_parser);
>>> +
>>> + DBG("wp_context->wispr_msg.message_type %d", wp_context->wispr_msg.message_type);
>>
>> We still use 80 character lines, so please break up accordingly.
>
> Surprisingly, checkpatch.pl did not squawk about this. Fixed, regardless.
>
The default is now 100 for the kernel. We're sticking to 80 for now. Maybe:
--max-line-length=n set the maximum line length, (default 100)
if exceeded, warn on patches
requires --strict for use with --file
works?
Regards,
-Denis