On Wed, 18 Sep 2013 16:04:28 +0200, Jakub Jermar <[email protected]> wrote:
On 17.9.2013 22:57, Dominik Taborsky wrote:
In that case, may I ask why there's USB error codes in errno.h?
It's not only USB error codes, but also file system and networking error
codes. However, we consider it a bad thing and you should not worsen the
situation.
OK.
I haven't had a chance to try running your code again. I've looked at
briefly and I still find some deviations from the official Cstyle,
such as:
(I am only including one exaple of each type of Cstyle error):
* Copyright (c) 2012, 2013 Dominik Taborsky
we only put the last year of modification in, such as:
* Copyright (c) 2013 Dominik Taborsky
I have to admit, though, that I am not sure whether the current way is
a
good idea, we might discuss about changing it
This is even more unimportant if you yourself don't know how it should
be done. So, am I expected to fix this to later find out the rule has
changed?
This is probably not a big issue and you will find variations here and
there.
OK.
input.h:
extern int get_input_line(tinput_t * in, char ** str);
extern uint8_t get_input_uint8(tinput_t * in);
extern uint32_t get_input_uint32(tinput_t * in);
extern uint64_t get_input_uint64(tinput_t * in);
extern size_t get_input_size_t(tinput_t * in); func_gpt.h:
extern int construct_gpt_label(label_t *);
extern int add_gpt_part (label_t *, tinput_t *);
extern int delete_gpt_part (label_t *, tinput_t *);
extern int destroy_gpt_label(label_t *);
extern int new_gpt_label (label_t *);
extern int print_gpt_parts (label_t *);
extern int read_gpt_parts (label_t *, service_id_t);
extern int write_gpt_parts (label_t *, service_id_t);
extern int extra_gpt_funcs (label_t *, tinput_t *, service_id_t);
Here's a block of function declarations that use tab for aligning - in
one
case you are aligning function names, in the other you are aligning the
opening brackets?!. You should not align either, use a single space
between
type and name (same for lists of variable declarations) and no space
between
name and opening bracket.
I am sorry about using tabs for alignment, I thought I fixed that
everywhere.
But I don't agree with you on your second point - not aligning argument
lists. I find that very handy and helpful. Looking up the function is
fast, but finding the opening bracket slows me down. Aligning them makes
it easy to spot differences.
It helps you to see that functions in general tend to have different
signatures? Most text editors with syntax highlighting will highlight
the brackets for you. And if you want to group a block of functions that
logically belong together, it is much better to use vertical spacing for
this purpose.
I don't know where you got that "most text editors" idea. I use Geany and
it doesn't do that. But that's not the point. The point is it is more
readable for "those less fortunate", if you will, who don't use "most text
editors". But if it is a problem, fine, I'll fix it.
As I said before, I make some exceptions to this rule. I know keeping
short lines allow you to open several source files at once and show them
side-by-side. But this is just a printf. It doesn't do anything
important. You don't need to care what's in there...
Please don't make deliberate exceptions to the rules, especially when
there is a nice and natural way to break the long statement. It doesn't
really matter what the function at hand does.
Fine.
The trouble is, if you name it "mbr.h", you might think it describes MBR
label only - not the library API as well. I think "libmbr.h" makes it
clear what it defines. Any other opinions?
The current mainline probably uses both ways (e.g. libfs.h for libfs and
minix.h for libminix), but IMO the latter prevails, therefore I would
suggest using mere mbr.h.
Are you sure? To me, mbr.h would only define 512-byte and 16-byte
structures and some constants. It wouldn't define a library. What if I
wanted to split those definitions into a separate file as was already
suggested? What would you name that file?
Again, as I said before, the first indentation is for readability
purposes. I'm sorry about the second one, must have slipped through...
It should nonetheless always be exactly 4 spaces.
Fine.
Sorry about those tabs. And my tab is 4 characters. BTW, I don't see how
"80 chars per line" and "8-char tab" can get together. Seems
contradicting to me...
You can have around 9 levels of indentation with these parameters, which
is still many more than any sane function would use.
Yes, but at that level of indentation you would have 8 chars left. That's
not really an argument, is it? If you have 4 level of indentation (which
is quite reasonable), 8-char tabs waste 16 chars for nothing. Thats one
fifth or 20% of the maximum space allowed per line. 20% wasted.
I don't really care how long tabs you have, and I'm not trying to argue
here. I just pointed out it seems silly to me.
That's not in the CStyle guide. And I prefer my style.
Perhaps it can be added there. But I don't think this is a matter of
style. It's just that the brackets are not necessary in this case.
If it's not in the official CStyle guide, how am I supposed to follow
rules? Jiri complained about me not following them, but how can I know?
/* Encoding and writing first logical partition */
if (l != &(label->parts->list.head)) {
had you used list_first()/list_next() you could check for l != NULL.
Explained before.
We also have some other code which does this, especially in the kernel.
I guess you can change it to be more readable as Jiri suggested or just
leave it as it is.
I can change it, and probably will. It's just I didn't notice those
functions before. But thanks.
CStyle guide reference?
The majority of the rest of the code does that. It's usually better to
write code which follows the same, possibly implied, conventions. It's
also not clear to me whether not having the empty line there would not
result in Doxygen inappropriately joining the \brief and \description.
Well, as I pointed earlier (in another thread about me not following
CStyle guide), there are places that break those rules already in
mainline. You noted how some libraries still use "lib" in their file
names, how there are USB error codes in errno.h and some deviations from
license formatting.
Don't take me wrong, but you can't expect me to follow rules, that are not
written (please don't try to apply this statement to outside world). Would
it be too much trouble to admit your rules are not complete and fix them
and then politely asking me/other devs to correct the code to apply to the
new rules, instead of arguing about it?
The alternative is to accept some slight and reasonable deviations from
your preferred style. I would prefer this route than the other, because it
just is more "humane".
You may have noticed that this very long e-mail (except for the
technical discussion at its end) is primarily about nitpicking your
deviations from the cstyle or from the style of the other code. I think
the cause which provokes an e-mail like this is rooted in contributors'
tendencies to make deliberate exceptions from various rules. It is easy
to say: "I don't like this and I'll do it my way, using my preferred
style." But it won't work in a multi-developer project like HelenOS. If
the contributors merely tried to comply as much as possible with the
rules, both written and implied, we could save ourselves some time. We
are being quite pedantic about it because it is clear that if the code
gets merged with cstyle issues, somebody will have to eventually fix
them and you can guess who that somebody will be. The other alternative
is that we end-up with as many cstyles as there are contributors, that
is: a complete chaos. If you think we are a sort of cstyle nazis, I
recommend you undergo a code inspection session or two in eg. Solaris
sustaining organization where exceptions from the cstyle are not
permitted at all.
1) Why would you think I consider you CStyle nazis?
2) I think I try to fix my code to your liking. You may notice that I'm
just asking for other opinions on uncertain issues.
3) I'm not adding code to kernel or any other part (except for
checksum.{c,h}) - I'm creating my own code from scratch. That lessens the
issues to a certain degree.
4) If you consider only extreme ways and your way, the discussion is not
worth it. I never said noone should ever follow any rules.
Dominik
_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/listinfo/helenos-devel
