[systemd-devel] Fragile journal interleaving

[Lennart Poettering]

On Do, 14.12.17 21:14, Uoti Urpala (uoti.urpala at pp1.inet.fi) wrote:

> > > +                if (j->current_location.type == LOCATION_DISCRETE)
> > > +                        // only == 0 or not matters
> > 
> > Please use C comments, /* */, see CODING_STYLE.
> 
> That is a standard C comment... IMO it's quite silly to insist on this
> kind of "This is not the C I first learned! I refuse to update the way
> I do things!" attitude for systemd. "//" comments and free placement of
> variable declarations are normal C and no more questionable than any
> other random feature. In practice, the ONLY reason anyone opposes them
> is that they were added at a later point. This is exactly the same kind
> of resistance to change that underlies most "sysvinit worked just fine
> for me, systemd is horrible" attitudes. So systemd in particular really
> should do better.

Well, the idea is that /* */ is used for explanatory comments and //
is left for local, temporary, non-commitable stuff.

i.e. if you are testing some stuff, and want to comment out some bits
briefly, use //, but if you add explanations for code that shall be
commited then please use /* */. That way you can see very easily what
is temporary stuff you should fix up before commiting and what should
go in.

different projects have different rules there. Some people use "FIXME"
or "XXX" for this purpose (and in fact its up to you if you do, after
all this kind of stuff should never end up being commited upstream),
but so far we just used C vs. C++ comments for this, and I think that
makes sense. 

> > > &j->current_location);
> > 
> > this returns an int, and you assign it to a bool. If the "int" was
> > actually used in a "boolean" way that would be OK, but it actually
> > returns < 0, 0, > 0, hence we should make the conversion explicit here
> > and write this as "found = c_w_l(…) != 0".
> 
> Well that int/bool distinction was basically why I added the comment
> above. I guess adding a redundant comparison is an alternative, matter
> of taste mostly...

Yeah, we try to not to rely too much on "magic" implicit C
casts, and try to use the language to clarify in code that we are aware of
the different types.

Lennart

-- 
Lennart Poettering, Red Hat