Re: [PATCH] Cygwin: pty: Add error handling in setup_pseudoconsoe().

[Corinna Vinschen]

On Feb 11 00:12, Takashi Yano wrote:
> - In setup_pseudoconsole(), many error handling was omitted. This
>   patch adds missing error handling.
> ---
>  winsup/cygwin/fhandler_tty.cc | 94 +++++++++++++++++++++++++----------
>  1 file changed, 68 insertions(+), 26 deletions(-)

Uhm... please, no.  There's no problem adding goto labels per se, but
jumping back to numbered error labels is quite confusing.

Error labels should ideally be at the end of the function and in reverse
order of the potentially failing code.  For more than one error label,
the label names ideally reflect the problem they are solving.  For
example:

  function()
  {
    <do something>

    if (<something failed>)
      goto err_something;

    <do something else>

    if (<something else failed>)
      goto err_something_else;

    <do some other stuff>

    if (<some other stuff failed>)
      goto err_some_other;
    [...]

    return true;

  err_some_other:
    <cleanup some other stuff>

  err_something_else:
    <cleanup something else>

  err_something:
    <cleanup something>

    return false;
  }

I wouldn't expect that all functions in Cygwin follow this approach yet,
but for new code I'd rather see it like this.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

signature.asc
Description: PGP signature