Re: [PATCH v4 5/9] wispr: Refactor 'wispr_portal_web_result' to leverage 'GError'.

[Denis Kenzior <denkenz@gmail.com>]

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