Bug 67620 - Code quality issue in weston?
Summary: Code quality issue in weston?
Status: RESOLVED INVALID
Alias: None
Product: Wayland
Classification: Unclassified
Component: weston (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: medium normal
Assignee: Wayland bug list
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-01 14:03 UTC by Rafael Castillo
Modified: 2013-08-06 15:33 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Rafael Castillo 2013-08-01 14:03:26 UTC
it was pointed out this function in a forum i read

handle_setmaster(struct weston_launch *wl, struct msghdr *msg, ssize_t len)

i saw this 
if (data->fd == -1) {
		error(0, 0, "missing drm fd in socket request");
		goto out; <----
	}

out: <--- this
	do { <--- this
		len = send(wl->sock[0], &ret, sizeof ret, 0);
	} while (len < 0 && errno == EINTR);

it seems to me very ugly but i understand weston is just a reference implementation too but in case someone wanna use it later would be nice to improve this, if you guide me a bit[im more of a c++ guy and not familiar with weston code] i can improve it
Comment 1 Rafael Castillo 2013-08-01 14:09:42 UTC
handle_open(struct weston_launch *wl, struct msghdr *msg, ssize_t len)

here is present too
Comment 2 Kristian Høgsberg 2013-08-04 20:45:16 UTC
(In reply to comment #1)
> handle_open(struct weston_launch *wl, struct msghdr *msg, ssize_t len)
> 
> here is present too

What is the problem you see?  Weston is the reference implementation, but that doesn't imply that lower standards or quality.
Comment 3 J 2013-08-05 12:17:26 UTC
While usage of 'goto' constantly receives a lot of negative press, it's usage in C code for error handling is generally considered cleaner than alternatives.

In my personal opinion, I find it easier to read as well - rather than multiple levels of scope/indentation based on every potential error, you get a normal flow of code, where the (rare) errors cause the flow to break. If they require the same cleanup code, we sure don't want any duplication either.

It's use should still be limited to specific applicable scenarios, but it's not a 'use a single goto and you'll cause a nuclear war' level style of issue. Hell, using a break in a switch statement is essentially a goto itself(!)
Comment 4 Rob Bradford 2013-08-06 15:33:27 UTC
Not enough information to resolve this issue.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.