On 09/25/2012 06:46 PM, Tanu Kaskinen wrote:
On Mon, 2012-09-24 at 17:02 +0200, David Henningsson wrote:
On 09/24/2012 03:33 PM, Tanu Kaskinen wrote:
Ok, I don't have a problem with this example. But the code in your patch
has different structure: instead of a simple match_found() call, the
inner code contains a three-line block.
Well, I still think
for (i)
for (j)
if (a[i] == b[j]) {
three();
line();
block();
}
...looks better than
for (i)
for (j)
if (a[i] == b[j]) {
three();
line();
block();
}
}
}
...but maybe that's just me.
I certainly prefer the latter, though this is not a very important thing
for me. But I'm now left wondering what you thought we agreed earlier -
unfortunately I don't have the logs, but I think we agreed to disallow
some cases of omitting the braces. If you think that the first example
would still be allowed, I don't know what case would be left to
disallow.
I don't recall for sure, but I think I was imagining multiline
statements, e g
if (i)
this_is_a_very_long_statement(with_multiple, parameters,
that_maybe_are_functions(themselves));
vs
if (i) {
this_is_a_very_long_statement(with_multiple, parameters,
that_maybe_are_functions(themselves));
}
in which case the latter is okay. Anyway, if it's not important, it's
just another trap to fall in when you try to fix somebody's real problem.
Fair enough. I don't like the check, because it implies that it's
possible that output_mappings is empty, which is not true, but I see how
this can prevent a bug in the future (not a severe bug, because only a
minor optimization would be skipped, but a bug anyway). I think it would
be best to always create the output_mappings idxset so that the code
doesn't have to worry about it being NULL. If you agree, I'll file a
bug, since the issue is separate from this patch and I don't plan to
write the patch right away, nor ask you to do so.
I don't know if we have a generic opinion on whether we prefer NULL
idxsets or empty idxsets? Or determine on a case-by-case basis?
I don't think there has been any rule so far, or if there has been, it
has been to favor NULL. I would definitely choose to always initialize
idxsets and hashmaps if there's no particular reason to distinguish
between NULL and an empty container. (I have even sent some patches that
ensure that certain idxsets or hashmaps are always non-NULL, but I don't
remember if they have been reviewed yet.) It makes the code simpler when
you don't have to worry about the container being NULL.
Another option could be to make pa_hashmap_* to treat NULL pointers as
empty containers. I don't know if that's better though.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
