All of lore.kernel.org
 help / color / mirror / Atom feed
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Mauro Carvalho Chehab <mchehab@infradead.org>,
	linux-media@vger.kernel.org
Cc: "Hans Verkuil" <hans.verkuil@cisco.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Andrey Utkin" <andrey_utkin@fastmail.com>,
	"Arvind Yadav" <arvind.yadav.cs@gmail.com>,
	"Bhumika Goyal" <bhumirks@gmail.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Brian Johnson" <brijohn@gmail.com>,
	"Christoph Böhmwalder" <christoph@boehmwalder.at>,
	"Christophe Jaillet" <christophe.jaillet@wanadoo.fr>,
	"Colin Ian King" <colin.king@canonical.com>,
	"Daniele Nicolodi" <daniele@grinta.net>,
	"David Härdeman" <david@hardeman.nu>,
	"Devendra Sharma" <devendra.sharma9091@gmail.com>,
	"Gustavo A. R. Silva" <garsilva@embeddedor.com>,
	"Inki Dae" <inki.dae@samsung.com>,
	"Joe Perches" <joe@perches.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Masahiro Yamada" <yamada.masahiro@socionext.com>,
	"Max Kellermann" <max.kellermann@gmail.com>,
	"Mike Isely" <isely@pobox.com>,
	"Philippe Ombredanne" <pombredanne@nexb.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Santosh Kumar Singh" <kumar.san1093@gmail.com>,
	"Satendra Singh Thakur" <satendra.t@samsung.com>,
	"Sean Young" <sean@mess.org>,
	"Seung-Woo Kim" <sw0312.kim@samsung.com>,
	"Shyam Saini" <mayhs11saini@gmail.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Todor Tomov" <todor.tomov@linaro.org>,
	"Wei Yongjun" <weiyongjun1@huawei.com>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org
Subject: Re: [v3] [media] Use common error handling code in 19 functions
Date: Sat, 05 May 2018 07:53:00 +0000	[thread overview]
Message-ID: <980b7bd9-b922-55f7-c2d7-2d20552ade4c@users.sourceforge.net> (raw)
In-Reply-To: <20180504144928.566ae507@vento.lan>

> @@ -656,18 +656,18 @@  static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
>  	tsfeed->priv = filter;
>  
>  	ret = tsfeed->set(tsfeed, feed->pid, ts_type, ts_pes, timeout);
> -	if (ret < 0) {
> -		dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto release_feed;
>  
>  	ret = tsfeed->start_filtering(tsfeed);
> -	if (ret < 0) {
> -		dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto release_feed;
>  
>  	return 0;
> +
> +release_feed:
> +	dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> +	return ret;
>  }
> 
> There's *nothing* wrong with the above. It works fine,

I can agree to this view in principle according to the required control flow.


> avoids goto

How does this wording fit to information from the section
“7) Centralized exiting of functions” in the document “coding-style.rst”?


> and probably even produce the same code, as gcc will likely optimize it.

Would you like to clarify the current situation around supported
software optimisations any more?


> It is also easier to review, as the error handling is closer
> to the code.

Do we stumble on different coding style preferences once more?


> On the other hand, there's nothing wrong on taking the approach
> you're proposing.

Thanks for another bit of positive feedback.


> In the end, using goto or not on error handling like the above is 
> a matter of personal taste - and taste changes with time

Do Linux guidelines need any adjustments?


> and with developer. I really don't have time to keep reviewing patches
> that are just churning the code just due to someone's personal taste.

I tried to apply another general source code transformation pattern.


> I'm pretty sure if I start accepting things like that,
> someone else would be on some future doing patches just reverting it,
> and I would be likely having to apply them too.

Why?

I hope also that the source code can be kept consistent to some degree.


> So, except if the patch is really fixing something - e.g. a broken
> error handling code, I'll just ignore such patches and mark as
> rejected without further notice/comments from now on.

I would find such a communication style questionable.
Do you distinguish between bug fixes and possible corrections for
other error categories (or software weaknesses)?

Regards,
Markus

WARNING: multiple messages have this Message-ID (diff)
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Mauro Carvalho Chehab <mchehab@infradead.org>,
	linux-media@vger.kernel.org
Cc: "Hans Verkuil" <hans.verkuil@cisco.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Andrey Utkin" <andrey_utkin@fastmail.com>,
	"Arvind Yadav" <arvind.yadav.cs@gmail.com>,
	"Bhumika Goyal" <bhumirks@gmail.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Brian Johnson" <brijohn@gmail.com>,
	"Christoph Böhmwalder" <christoph@boehmwalder.at>,
	"Christophe Jaillet" <christophe.jaillet@wanadoo.fr>,
	"Colin Ian King" <colin.king@canonical.com>,
	"Daniele Nicolodi" <daniele@grinta.net>,
	"David Härdeman" <david@hardeman.nu>,
	"Devendra Sharma" <devendra.sharma9091@gmail.com>,
	"Gustavo A. R. Silva" <garsilva@embeddedor.com>,
	"Inki Dae" <inki.dae@samsung.com>,
	"Joe Perches" <joe@perches.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Masahiro Yamada" <yamada.masahiro@socionext.com>,
	"Max Kellermann" <max.kellermann@gmail.com>,
	"Mike Isely" <isely@pobox.com>,
	"Philippe Ombredanne" <pombredanne@nexb.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Santosh Kumar Singh" <kumar.san1093@gmail.com>,
	"Satendra Singh Thakur" <satendra.t@samsung.com>,
	"Sean Young" <sean@mess.org>,
	"Seung-Woo Kim" <sw0312.kim@samsung.com>,
	"Shyam Saini" <mayhs11saini@gmail.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Todor Tomov" <todor.tomov@linaro.org>,
	"Wei Yongjun" <weiyongjun1@huawei.com>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org
Subject: Re: [v3] [media] Use common error handling code in 19 functions
Date: Sat, 5 May 2018 09:53:00 +0200	[thread overview]
Message-ID: <980b7bd9-b922-55f7-c2d7-2d20552ade4c@users.sourceforge.net> (raw)
In-Reply-To: <20180504144928.566ae507@vento.lan>

> @@ -656,18 +656,18 @@  static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
>  	tsfeed->priv = filter;
>  
>  	ret = tsfeed->set(tsfeed, feed->pid, ts_type, ts_pes, timeout);
> -	if (ret < 0) {
> -		dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto release_feed;
>  
>  	ret = tsfeed->start_filtering(tsfeed);
> -	if (ret < 0) {
> -		dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto release_feed;
>  
>  	return 0;
> +
> +release_feed:
> +	dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> +	return ret;
>  }
> 
> There's *nothing* wrong with the above. It works fine,

I can agree to this view in principle according to the required control flow.


> avoids goto

How does this wording fit to information from the section
“7) Centralized exiting of functions” in the document “coding-style.rst”?


> and probably even produce the same code, as gcc will likely optimize it.

Would you like to clarify the current situation around supported
software optimisations any more?


> It is also easier to review, as the error handling is closer
> to the code.

Do we stumble on different coding style preferences once more?


> On the other hand, there's nothing wrong on taking the approach
> you're proposing.

Thanks for another bit of positive feedback.


> In the end, using goto or not on error handling like the above is 
> a matter of personal taste - and taste changes with time

Do Linux guidelines need any adjustments?


> and with developer. I really don't have time to keep reviewing patches
> that are just churning the code just due to someone's personal taste.

I tried to apply another general source code transformation pattern.


> I'm pretty sure if I start accepting things like that,
> someone else would be on some future doing patches just reverting it,
> and I would be likely having to apply them too.

Why?

I hope also that the source code can be kept consistent to some degree.


> So, except if the patch is really fixing something - e.g. a broken
> error handling code, I'll just ignore such patches and mark as
> rejected without further notice/comments from now on.

I would find such a communication style questionable.
Do you distinguish between bug fixes and possible corrections for
other error categories (or software weaknesses)?

Regards,
Markus

  reply	other threads:[~2018-05-05  7:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-19 18:11 [PATCH v2] [media] Use common error handling code in 20 functions SF Markus Elfring
2018-02-19 18:11 ` SF Markus Elfring
2018-02-19 18:37 ` Laurent Pinchart
2018-02-19 18:37   ` Laurent Pinchart
2018-02-28  8:45   ` [v2] " SF Markus Elfring
2018-02-28  8:45     ` SF Markus Elfring
2018-02-28  8:59     ` Laurent Pinchart
2018-02-28  8:59       ` Laurent Pinchart
2018-02-20  8:07 ` [PATCH v2] " Todor Tomov
2018-02-20  8:07   ` Todor Tomov
2018-03-06 17:08 ` [v2] " SF Markus Elfring
2018-03-06 17:08   ` SF Markus Elfring
2018-03-09 20:10 ` [PATCH v3] [media] Use common error handling code in 19 functions SF Markus Elfring
2018-03-09 20:10   ` SF Markus Elfring
2018-03-12  9:30   ` Todor Tomov
2018-03-12  9:30     ` Todor Tomov
2018-05-04 16:08   ` [v3] " SF Markus Elfring
2018-05-04 16:08     ` SF Markus Elfring
2018-05-04 17:50     ` Mauro Carvalho Chehab
2018-05-04 17:50       ` Mauro Carvalho Chehab
2018-05-05  7:53       ` SF Markus Elfring [this message]
2018-05-05  7:53         ` SF Markus Elfring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=980b7bd9-b922-55f7-c2d7-2d20552ade4c@users.sourceforge.net \
    --to=elfring@users.sourceforge.net \
    --cc=akpm@linux-foundation.org \
    --cc=andrey_utkin@fastmail.com \
    --cc=arvind.yadav.cs@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=bhumirks@gmail.com \
    --cc=brijohn@gmail.com \
    --cc=christoph@boehmwalder.at \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=colin.king@canonical.com \
    --cc=daniele@grinta.net \
    --cc=david@hardeman.nu \
    --cc=devendra.sharma9091@gmail.com \
    --cc=garsilva@embeddedor.com \
    --cc=hans.verkuil@cisco.com \
    --cc=inki.dae@samsung.com \
    --cc=isely@pobox.com \
    --cc=joe@perches.com \
    --cc=keescook@chromium.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kumar.san1093@gmail.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=max.kellermann@gmail.com \
    --cc=mayhs11saini@gmail.com \
    --cc=mchehab@infradead.org \
    --cc=mchehab@kernel.org \
    --cc=pombredanne@nexb.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=satendra.t@samsung.com \
    --cc=sean@mess.org \
    --cc=sw0312.kim@samsung.com \
    --cc=tglx@linutronix.de \
    --cc=todor.tomov@linaro.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=weiyongjun1@huawei.com \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.