30 Mar
2016
30 Mar
'16
7:47 p.m.
On Wednesday 30 March 2016 15:31:27 Fons Adriaensen wrote:
On Tue, Mar 29, 2016 at 11:12:21AM -0400, Paul Davis
wrote:
Phew! I'm with you 100% Fons. Stay upwind, way upwind of code like this.
For starters, where the heck are the closing } for the two else clauses
above?
Cheers, Gene Heskett
--
"There are four boxes to be used in defense of liberty:
soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
Genes Web page <http://geneslinuxbox.net:6309/gene>
(1) I explained to Fons in person and to everyon
on the jack-devel
mailing list what the issues with Fons' original patch. Robin Gareus
has eloquently restated the basic point, but I'll do it again: the
ability to REVIEW code changes is critical to long-lived projects,
and when a change consists of 85% or more white-space/code style
changes and only 15% actual functional differences, code review
becomes all but impossible.
1. All the places where the actual code was changed were
clearly identified by comments.
2. When I compare the commit (42393121) that applied my patch
with previous versions, there are *lots of* major coding
style and whitespace changes in files that I never touched.
This means the coding style wasn't consistent to start with.
3. When I read that same commit (which I assume reflects the
preferred style) I see things like this:
engine.c, line 3884:
-----
if (dstport->connections && !dstport->shared->has_mixdown) {
jack_port_type_info_t *port_type =
jack_port_type_info (engine, dstport);
jack_error ("cannot make multiple connections to a
port of" " type [%s]", port_type->type_name); return -1;
}
connection = (jack_connection_internal_t*)
malloc (sizeof(jack_connection_internal_t));
connection->source = srcport;
connection->destination = dstport;
connection->srcclient = srcclient;
connection->dstclient = dstclient;
src_id = srcport->shared->id;
dst_id = dstport->shared->id;
jack_lock_graph (engine);
if (dstclient->control->type == ClientDriver) {
/* Ignore output connections to drivers for
purposes of sorting. Drivers are executed first in the sort order
anyway, and we don't want to treat graphs such as driver -> client ->
driver as containing feedback */
VERBOSE (engine,
"connect %s and %s (output)",
srcport->shared->name,
dstport->shared->name);
} else if (srcclient != dstclient) {
jack_feedcounts_t *F;
int dir = 0;
if ((F = depends_direct (srcclient, dstclient)) != 0)
{ /* We already have a direct dependency, just increment the
connection count. */ F->fwd_count++;
dir = 1;
} else if ((F = depends_direct (dstclient, srcclient))
!= 0) { /* We already have a reverse direct dependency, just increment
the reverse connection count. */ F->rev_count++;
dir = -1;
----
If this is your idea of indentation that is supposed to help
understand the code structure, then I'm lost. This sort of thing is
why I modify whitespace when working on a file, just to be sure I have
the correct idea of where conditionals and loops start and end. The
original code was full of random indentation like this.
Ciao,