On Wed, Feb 02, 2022 at 08:17:53AM +0100, Michal Prívozník wrote:
On 2/2/22 01:06, Martin Kletzander wrote:On Tue, Feb 01, 2022 at 05:23:33PM +0100, Tim Wiederhake wrote:On Tue, 2022-02-01 at 16:07 +0000, Daniel P. Berrangé wrote:On Tue, Feb 01, 2022 at 03:25:55PM +0100, Martin Kletzander wrote: > On Tue, Feb 01, 2022 at 02:20:17PM +0100, Tim Wiederhake wrote: > > Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx> > > --- > > src/lxc/lxc_driver.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c > > index 7bc39120ee..42053de9c3 100644 > > --- a/src/lxc/lxc_driver.c > > +++ b/src/lxc/lxc_driver.c > > @@ -4045,9 +4045,9 @@ > > lxcDomainDetachDeviceHostdevUSBLive(virLXCDriver *driver, > > VIR_WARN("cannot deny device %s for domain %s: %s", > > dst, vm->def->name, virGetLastErrorMessage()); > > > > - virObjectLock(hostdev_mgr->activeUSBHostdevs); > > - virUSBDeviceListDel(hostdev_mgr->activeUSBHostdevs, usb); > > - virObjectUnlock(hostdev_mgr->activeUSBHostdevs); > > + VIR_WITH_OBJECT_LOCK_GUARD(hostdev_mgr->activeUSBHostdevs) { > > + virUSBDeviceListDel(hostdev_mgr->activeUSBHostdevs, > > usb); > > + } > > Even nicer you can omit the curly brackets and make this a +2/-3 ;) > Otherwise I guess we'd need some kind of stylistic guide to keep us > consistent in this regard right from the start. My preference is the keep the scope of the guarded section explicit using {} even with a 1 line body.Same for me, fwiw.OK, I don't really have a horse in this race. But in that case it would be nice to document it in our guidelines, maybe together with few words around the new macros. In a later patch for example.Since you brought this up, I vaguely remember us discussing this earlier, though it was for one line body of if()-s. I too am in favor of having curly braces in that case, except maybe when the body is a goto or return statement. IOW: if (cond) return -1; // good if (cond) { func(); // good } if (cond) { goto cleanup; // not a big fan } OTOH, consistency matters more so I can live with the last example too.
It depends how much we like consistency, right? I used to prefer curly brackets around any sub-statement, even one-liners, but then I started contributing to libvirt and had to adapt. If we want to be 100% consistent then it would make sense to just have them all the time even though I do not prefer that style any more, but I would adapt. Everyone would, I think. And that leads me to another idea which I mentioned in the past already. I don't want to repeat myself too much, but "since you bought this up" and this is very related, here goes nothing. All this talk about "which subjective way of writing the same code should we choose" seems a bit of a moo point since even though we always mention consistency we end up choosing what we like better. But since no decision will cater to everyone and everyone will, at some point, have to adapt, maybe there is a better solution to the problem. Maybe the solution is even more consistency so that there are fewer exceptions. So what if we find another advantage/reason for deciding on a way to format the code? One possible advantage would be that if we choose to comply with a format that can be easily enforced (something that indent / clang-format / whatever can do a good job of) we could erase lot of docs, lot of syntax checking code, reduce the knowledge required for newcomers to send proper code, make it easier to automatically fix code to comply to the rules, and so on. The only disadvantage would be that most of us would have to adapt and my point is that we have to do that anyway (new syntax, new "libvirt language", working on other projects) and more consistency just makes it even easier IMHO, even if it takes a bit longer. Anyway, just something I felt like mentioning Martin
Attachment:
signature.asc
Description: PGP signature
