Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

Dan Carpenter  writes:
> Compare:
>
>   foo = kmalloc(sizeof(*foo), GFP_KERNEL);
>
> This says you are allocating enough space for foo.  It can be reviewed
> by looking at one line.  If you change the type of foo it will still
> work.
>
>   foo = kmalloc(sizeof(struct whatever), GFP_KERNEL);
>
> There isn't enough information to say if this is correct.  If you change
> the type of foo then you have to update the allocation as well.
>
> It's not a super common type of bug, but I see it occasionally.

I know what you are saying, but the latter in my book is easier to read
and reminds you what the type is when you review the code.

Point being this comes down to personal preference and stating that the
former is the right way or making that a rule and using checkpatch to
harrass people with patches to change it is bogus.

Jes

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

Dan Carpenter  writes:
> Compare:
>
>   foo = kmalloc(sizeof(*foo), GFP_KERNEL);
>
> This says you are allocating enough space for foo.  It can be reviewed
> by looking at one line.  If you change the type of foo it will still
> work.
>
>   foo = kmalloc(sizeof(struct whatever), GFP_KERNEL);
>
> There isn't enough information to say if this is correct.  If you change
> the type of foo then you have to update the allocation as well.
>
> It's not a super common type of bug, but I see it occasionally.

I know what you are saying, but the latter in my book is easier to read
and reminds you what the type is when you review the code.

Point being this comes down to personal preference and stating that the
former is the right way or making that a rule and using checkpatch to
harrass people with patches to change it is bogus.

Jes

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Dan Carpenter]

Compare:

foo = kmalloc(sizeof(*foo), GFP_KERNEL);

This says you are allocating enough space for foo.  It can be reviewed
by looking at one line.  If you change the type of foo it will still
work.

foo = kmalloc(sizeof(struct whatever), GFP_KERNEL);

There isn't enough information to say if this is correct.  If you change
the type of foo then you have to update the allocation as well.

It's not a super common type of bug, but I see it occasionally.

regards,
dan carpenter

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Dan Carpenter]

Compare:

foo = kmalloc(sizeof(*foo), GFP_KERNEL);

This says you are allocating enough space for foo.  It can be reviewed
by looking at one line.  If you change the type of foo it will still
work.

foo = kmalloc(sizeof(struct whatever), GFP_KERNEL);

There isn't enough information to say if this is correct.  If you change
the type of foo then you have to update the allocation as well.

It's not a super common type of bug, but I see it occasionally.

regards,
dan carpenter

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Dan Carpenter]

Oops.  I sent this email twice.  Sorry, about that.

regards,
dan carpenter

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Dan Carpenter]

Oops.  I sent this email twice.  Sorry, about that.

regards,
dan carpenter

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>> Can it "accidentally" happen that some of them will be really worth
>> also for your precious software development attention?
> 
> Given that none of your patches fix any real bugs

Are there any ones which would eventually become "real" also for you?


> and you do your best to ignore any guidance you have been given,

I dare occasionally to find reasons out for a specific disagreement.


> I do reject your entire patchset and you can consider this a NACK for this 
> entire series.

Thanks for your feedback.

I am still curious if any other software developers or source code reviewers
would dare to express an other opinion for one of the shown update 
possibilities.

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>> Can it "accidentally" happen that some of them will be really worth
>> also for your precious software development attention?
> 
> Given that none of your patches fix any real bugs

Are there any ones which would eventually become "real" also for you?


> and you do your best to ignore any guidance you have been given,

I dare occasionally to find reasons out for a specific disagreement.


> I do reject your entire patchset and you can consider this a NACK for this 
> entire series.

Thanks for your feedback.

I am still curious if any other software developers or source code reviewers
would dare to express an other opinion for one of the shown update 
possibilities.

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

SF Markus Elfring  writes:
>>> How do you value compliance with coding styles?
>>
>> The Linux Coding Style is not a law,
>
> How serious can such guidelines become for software developers?
>
>> nor is it at all perfect.
>
> I got a similar impression. But are there enough items where a mostly clear
> guidance is specified?
>
>> You clearly misunderstood how Linux development work
>
> I got an other impression.
>
>> and you are doing a great job wasting everyone's time with this patchset.
>
> Would you like to reject any update steps for the affected source files
> from this patch series?
>
> Can it "accidentally" happen that some of them will be really worth
> also for your precious software development attention?

Given that none of your patches fix any real bugs and you do your best
to ignore any guidance you have been given, I do reject your entire
patchset and you can consider this a NACK for this entire series.

I get the impression you obtain your response to any email from M-x
doctor.

Jes

Re: md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

SF Markus Elfring  writes:
>>> How do you value compliance with coding styles?
>>
>> The Linux Coding Style is not a law,
>
> How serious can such guidelines become for software developers?
>
>> nor is it at all perfect.
>
> I got a similar impression. But are there enough items where a mostly clear
> guidance is specified?
>
>> You clearly misunderstood how Linux development work
>
> I got an other impression.
>
>> and you are doing a great job wasting everyone's time with this patchset.
>
> Would you like to reject any update steps for the affected source files
> from this patch series?
>
> Can it "accidentally" happen that some of them will be really worth
> also for your precious software development attention?

Given that none of your patches fix any real bugs and you do your best
to ignore any guidance you have been given, I do reject your entire
patchset and you can consider this a NACK for this entire series.

I get the impression you obtain your response to any email from M-x
doctor.

Jes

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Bjørn Mork]

Dan Carpenter  writes:

> I am ignoring Markus patches and have told him that he should focus on
> bug fixes.  These patches don't add any value and regularly introduce
> bugs.

I think there should be a big fat warning in CodingStyle:

 THIS DOCUMENT DOES NOT APPLY TO ANY EXISTING KERNEL CODE.


Preserving existing style is more important than any minor style issue
anyway.  Style changes for the only purpose of style change should be
considered harmful and destructive behaviour. You'd think that is bloody
obvious, but evidently it isn't.  Not only are the untested, buggy,
churning patches still being submitted - some of them are even applied!

Adding such a statement will not prevent style changes, even changes
to bring code more in line with CodingStyle, as long as the patches are
part of some work of substance.  E.g. cleaning up before fixing bugs. If
you submit new code, then you can of course fix the style of any touched
context.

Oh well, I can dream..


Bjørn

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Bjørn Mork]

Dan Carpenter  writes:

> I am ignoring Markus patches and have told him that he should focus on
> bug fixes.  These patches don't add any value and regularly introduce
> bugs.

I think there should be a big fat warning in CodingStyle:

 THIS DOCUMENT DOES NOT APPLY TO ANY EXISTING KERNEL CODE.


Preserving existing style is more important than any minor style issue
anyway.  Style changes for the only purpose of style change should be
considered harmful and destructive behaviour. You'd think that is bloody
obvious, but evidently it isn't.  Not only are the untested, buggy,
churning patches still being submitted - some of them are even applied!

Adding such a statement will not prevent style changes, even changes
to bring code more in line with CodingStyle, as long as the patches are
part of some work of substance.  E.g. cleaning up before fixing bugs. If
you submit new code, then you can of course fix the style of any touched
context.

Oh well, I can dream..


Bjørn

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>> How do you value compliance with coding styles?
>
> The Linux Coding Style is not a law,

How serious can such guidelines become for software developers?


> nor is it at all perfect.

I got a similar impression. But are there enough items where a mostly clear
guidance is specified?


> You clearly misunderstood how Linux development work

I got an other impression.


> and you are doing a great job wasting everyone's time with this patchset.

Would you like to reject any update steps for the affected source files
from this patch series?

Can it "accidentally" happen that some of them will be really worth
also for your precious software development attention?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>> How do you value compliance with coding styles?
>
> The Linux Coding Style is not a law,

How serious can such guidelines become for software developers?


> nor is it at all perfect.

I got a similar impression. But are there enough items where a mostly clear
guidance is specified?


> You clearly misunderstood how Linux development work

I got an other impression.


> and you are doing a great job wasting everyone's time with this patchset.

Would you like to reject any update steps for the affected source files
from this patch series?

Can it "accidentally" happen that some of them will be really worth
also for your precious software development attention?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

SF Markus Elfring  writes:
 but patches that just fix coding style are a bad thing
>>>
>>> When you find such a change opportunity so "bad", are there any 
>>> circumstances left over where you would dare to touch the corresponding 
>>> source code line.
>> 
>> If you actually rewrite the code or fix some real bug there.
>
> Do the proposed update steps 12 - 16 for the function "setup_conf"
> (in this software module) fit to your condition?
>
> Do you reject this update step?

I do - those changes do nothing to improve the code and simply hides a
lot of history.

Jes

Re: md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

SF Markus Elfring  writes:
 but patches that just fix coding style are a bad thing
>>>
>>> When you find such a change opportunity so "bad", are there any 
>>> circumstances left over where you would dare to touch the corresponding 
>>> source code line.
>> 
>> If you actually rewrite the code or fix some real bug there.
>
> Do the proposed update steps 12 - 16 for the function "setup_conf"
> (in this software module) fit to your condition?
>
> Do you reject this update step?

I do - those changes do nothing to improve the code and simply hides a
lot of history.

Jes

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

Dan Carpenter  writes:
> On Thu, Oct 06, 2016 at 11:29:20AM +0200, Richard Weinberger wrote:
>> On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
>>  wrote:
>> > From: Markus Elfring 
>> > Date: Tue, 4 Oct 2016 21:46:18 +0200
>> >
>> > Replace the specification of a data structure by a pointer dereference
>> > as the parameter for the operator "sizeof" to make the corresponding size
>> > determination a bit safer.
>> 
>> Isn't this pure matter of taste?
>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>> developers prefer sizeof(struct foo) because you can determine the type
>> at first sight and makes review more easy.
>
> sizeof(*ptr) is more future proof and normally more obvious and easier
> to review.  That said, I've tried to tell Markus to only send bugfix
> patches because these are a waste of time and regularly introduce bugs.

This is totally a matter of taste. I for one find it way easier to
review something which says 'sizeof(struct )' because it stands out
more. I am curious what you mean by it being more future proof - if the
code says 'struct foo' in the sizeof argument, what is the problem?

The one area where there is a higher risk is if the type is changed, but
that is outweighed by the fact the spelled out version is easier to
review.

Jes

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

Dan Carpenter  writes:
> On Thu, Oct 06, 2016 at 11:29:20AM +0200, Richard Weinberger wrote:
>> On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
>>  wrote:
>> > From: Markus Elfring 
>> > Date: Tue, 4 Oct 2016 21:46:18 +0200
>> >
>> > Replace the specification of a data structure by a pointer dereference
>> > as the parameter for the operator "sizeof" to make the corresponding size
>> > determination a bit safer.
>> 
>> Isn't this pure matter of taste?
>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>> developers prefer sizeof(struct foo) because you can determine the type
>> at first sight and makes review more easy.
>
> sizeof(*ptr) is more future proof and normally more obvious and easier
> to review.  That said, I've tried to tell Markus to only send bugfix
> patches because these are a waste of time and regularly introduce bugs.

This is totally a matter of taste. I for one find it way easier to
review something which says 'sizeof(struct )' because it stands out
more. I am curious what you mean by it being more future proof - if the
code says 'struct foo' in the sizeof argument, what is the problem?

The one area where there is a higher risk is if the type is changed, but
that is outweighed by the fact the spelled out version is easier to
review.

Jes

Re: md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

SF Markus Elfring  writes:
 Replace the specification of a data structure by a pointer dereference
 as the parameter for the operator "sizeof" to make the corresponding size
 determination a bit safer.
>>>
>>> Isn't this pure matter of taste?
>>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>>> developers prefer sizeof(struct foo) because you can determine the type
>>> at first sight and makes review more easy.
>> 
>> sizeof(*ptr) is more future proof and normally more obvious and easier
>> to review.
>
> Is it interesting to see how different the software development opinions
> can be for such an implementation detail?
>
>> That said, I've tried to tell Markus to only send bugfix patches
>
> Can any deviations from the Linux coding style become "bugs" also in
> your view of the software situation?
>
>> because these are a waste of time
>
> How do you value compliance with coding styles?

The Linux Coding Style is not a law, nor is it at all perfect. You
clearly misunderstood how Linux development work and you are doing a
great job wasting everyone's time with this patchset.

Jes

Re: md/raid1: Improve another size determination in setup_conf()

[Jes Sorensen]

SF Markus Elfring  writes:
 Replace the specification of a data structure by a pointer dereference
 as the parameter for the operator "sizeof" to make the corresponding size
 determination a bit safer.
>>>
>>> Isn't this pure matter of taste?
>>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>>> developers prefer sizeof(struct foo) because you can determine the type
>>> at first sight and makes review more easy.
>> 
>> sizeof(*ptr) is more future proof and normally more obvious and easier
>> to review.
>
> Is it interesting to see how different the software development opinions
> can be for such an implementation detail?
>
>> That said, I've tried to tell Markus to only send bugfix patches
>
> Can any deviations from the Linux coding style become "bugs" also in
> your view of the software situation?
>
>> because these are a waste of time
>
> How do you value compliance with coding styles?

The Linux Coding Style is not a law, nor is it at all perfect. You
clearly misunderstood how Linux development work and you are doing a
great job wasting everyone's time with this patchset.

Jes

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

> I am ignoring Markus patches

It's a pity that you chose such a reaction.


> and have told him that he should focus on bug fixes.

I find that I suggest to improve something. Could you admit a few times
that I found a "bug" you care also about at other source code places?


> These patches don't add any value

Can it be that you express a lower value for the Linux coding style here
than desired as there might be other concerns behind such negative feedback?


> and regularly introduce bugs.

How do you think about to discuss corresponding facts further?

 
> That said, "sizeof(*ptr)" is sort of official style.

When this implementation detail is so official, I wonder then why some
software developers can become "special" about the proposed update step
like for this module.

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

> I am ignoring Markus patches

It's a pity that you chose such a reaction.


> and have told him that he should focus on bug fixes.

I find that I suggest to improve something. Could you admit a few times
that I found a "bug" you care also about at other source code places?


> These patches don't add any value

Can it be that you express a lower value for the Linux coding style here
than desired as there might be other concerns behind such negative feedback?


> and regularly introduce bugs.

How do you think about to discuss corresponding facts further?

 
> That said, "sizeof(*ptr)" is sort of official style.

When this implementation detail is so official, I wonder then why some
software developers can become "special" about the proposed update step
like for this module.

Regards,
Markus

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Dan Carpenter]

On Thu, Oct 06, 2016 at 11:29:20AM +0200, Richard Weinberger wrote:
> On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
>  wrote:
> > From: Markus Elfring 
> > Date: Tue, 4 Oct 2016 21:46:18 +0200
> >
> > Replace the specification of a data structure by a pointer dereference
> > as the parameter for the operator "sizeof" to make the corresponding size
> > determination a bit safer.
> 
> Isn't this pure matter of taste?
> Some developers prefer sizeof(*ptr) because it is easier to type, other
> developers prefer sizeof(struct foo) because you can determine the type
> at first sight and makes review more easy.
> 

I am ignoring Markus patches and have told him that he should focus on
bug fixes.  These patches don't add any value and regularly introduce
bugs.

That said, "sizeof(*ptr)" is sort of official style.  It's slightly
more obvious and easier to review because all the information you need
is on that one line.  Also if we change the datatype of ptr then that
format is slightly more future proof.

regards,
dan carpenter

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Dan Carpenter]

On Thu, Oct 06, 2016 at 11:29:20AM +0200, Richard Weinberger wrote:
> On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
>  wrote:
> > From: Markus Elfring 
> > Date: Tue, 4 Oct 2016 21:46:18 +0200
> >
> > Replace the specification of a data structure by a pointer dereference
> > as the parameter for the operator "sizeof" to make the corresponding size
> > determination a bit safer.
> 
> Isn't this pure matter of taste?
> Some developers prefer sizeof(*ptr) because it is easier to type, other
> developers prefer sizeof(struct foo) because you can determine the type
> at first sight and makes review more easy.
> 

I am ignoring Markus patches and have told him that he should focus on
bug fixes.  These patches don't add any value and regularly introduce
bugs.

That said, "sizeof(*ptr)" is sort of official style.  It's slightly
more obvious and easier to review because all the information you need
is on that one line.  Also if we change the datatype of ptr then that
format is slightly more future proof.

regards,
dan carpenter

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>>> but patches that just fix coding style are a bad thing
>>
>> When you find such a change opportunity so "bad", are there any 
>> circumstances left over where you would dare to touch the corresponding 
>> source code line.
> 
> If you actually rewrite the code or fix some real bug there.

Do the proposed update steps 12 - 16 for the function "setup_conf"
(in this software module) fit to your condition?

Do you reject this update step?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>>> but patches that just fix coding style are a bad thing
>>
>> When you find such a change opportunity so "bad", are there any 
>> circumstances left over where you would dare to touch the corresponding 
>> source code line.
> 
> If you actually rewrite the code or fix some real bug there.

Do the proposed update steps 12 - 16 for the function "setup_conf"
(in this software module) fit to your condition?

Do you reject this update step?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[Jiri Kosina]

On Fri, 7 Oct 2016, SF Markus Elfring wrote:

> > but patches that just fix coding style are a bad thing
> 
> When you find such a change opportunity so "bad", are there any 
> circumstances left over where you would dare to touch the corresponding 
> source code line.

If you actually rewrite the code or fix some real bug there.

> > because they break things like `git blame`
> 
> I follow your concern to some degree.
> 
> But can this argument evolve against a lot of changes generally?

If I have to reiterate git blame multiple times just because of whitespace 
or codingstyle changes, it's a pure waste of my time.

If I have to reiterate git blame multiple times to skip actual real 
changes, I have no other option than to live with that (because there was 
an actual functional reason for the change).

-- 
Jiri Kosina
SUSE Labs

Re: md/raid1: Improve another size determination in setup_conf()

[Jiri Kosina]

On Fri, 7 Oct 2016, SF Markus Elfring wrote:

> > but patches that just fix coding style are a bad thing
> 
> When you find such a change opportunity so "bad", are there any 
> circumstances left over where you would dare to touch the corresponding 
> source code line.

If you actually rewrite the code or fix some real bug there.

> > because they break things like `git blame`
> 
> I follow your concern to some degree.
> 
> But can this argument evolve against a lot of changes generally?

If I have to reiterate git blame multiple times just because of whitespace 
or codingstyle changes, it's a pure waste of my time.

If I have to reiterate git blame multiple times to skip actual real 
changes, I have no other option than to live with that (because there was 
an actual functional reason for the change).

-- 
Jiri Kosina
SUSE Labs

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>> Why do various software developers bother about coding style specifications
>> at all then?
> Coding style is important,

Thanks that you "dare" to express also such an opinion.


> but patches that just fix coding style are a bad thing

When you find such a change opportunity so "bad", are there any circumstances
left over where you would dare to touch the corresponding source code line.


> because they break things like `git blame`

I follow your concern to some degree.

But can this argument evolve against a lot of changes generally?


> and run the risk of introducing new bugs

Did this really "happen" because of an update suggestion for this software 
module?


> without any net benefit to end users.

Can the proposed adjustment help to make a function like "setup_conf"
a bit more robust (together with related update steps) so that an improved
coding style compliance will hopefully influence the error probability
in positive ways?


> This goes double for code you don't actually work on regularly
> or don't completely understand.

How does such a kind of general feedback fit to the shown change
possibilities in this patch series?

Do you reject this update step?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>> Why do various software developers bother about coding style specifications
>> at all then?
> Coding style is important,

Thanks that you "dare" to express also such an opinion.


> but patches that just fix coding style are a bad thing

When you find such a change opportunity so "bad", are there any circumstances
left over where you would dare to touch the corresponding source code line.


> because they break things like `git blame`

I follow your concern to some degree.

But can this argument evolve against a lot of changes generally?


> and run the risk of introducing new bugs

Did this really "happen" because of an update suggestion for this software 
module?


> without any net benefit to end users.

Can the proposed adjustment help to make a function like "setup_conf"
a bit more robust (together with related update steps) so that an improved
coding style compliance will hopefully influence the error probability
in positive ways?


> This goes double for code you don't actually work on regularly
> or don't completely understand.

How does such a kind of general feedback fit to the shown change
possibilities in this patch series?

Do you reject this update step?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[Austin S. Hemmelgarn]

On 2016-10-07 06:50, SF Markus Elfring wrote:

Linux has tons of issues, fixes for real problems are very welcome.


Is a spectrum of software improvements to reconsider there?



But coding style bike shedding is just a waste of time.


Why do various software developers bother about coding style specifications
at all then?
Coding style is important, but patches that just fix coding style are a 
bad thing because they break things like `git blame` and run the risk of 
introducing new bugs without any net benefit to end users.  This goes 
double for code you don't actually work on regularly or don't completely 
understand.

Re: md/raid1: Improve another size determination in setup_conf()

[Austin S. Hemmelgarn]

On 2016-10-07 06:50, SF Markus Elfring wrote:

Linux has tons of issues, fixes for real problems are very welcome.


Is a spectrum of software improvements to reconsider there?



But coding style bike shedding is just a waste of time.


Why do various software developers bother about coding style specifications
at all then?
Coding style is important, but patches that just fix coding style are a 
bad thing because they break things like `git blame` and run the risk of 
introducing new bugs without any net benefit to end users.  This goes 
double for code you don't actually work on regularly or don't completely 
understand.

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

> Linux has tons of issues, fixes for real problems are very welcome.

Is a spectrum of software improvements to reconsider there?


> But coding style bike shedding is just a waste of time.

Why do various software developers bother about coding style specifications
at all then?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

> Linux has tons of issues, fixes for real problems are very welcome.

Is a spectrum of software improvements to reconsider there?


> But coding style bike shedding is just a waste of time.

Why do various software developers bother about coding style specifications
at all then?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[Richard Weinberger]

On 07.10.2016 10:53, SF Markus Elfring wrote:
 Replace the specification of a data structure by a pointer dereference
 as the parameter for the operator "sizeof" to make the corresponding size
 determination a bit safer.
>>>
>>> Isn't this pure matter of taste?
>>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>>> developers prefer sizeof(struct foo) because you can determine the type
>>> at first sight and makes review more easy.
>>
>> sizeof(*ptr) is more future proof and normally more obvious and easier
>> to review.
> 
> Is it interesting to see how different the software development opinions
> can be for such an implementation detail?
> 
> 
>> That said, I've tried to tell Markus to only send bugfix patches
> 
> Can any deviations from the Linux coding style become "bugs" also in
> your view of the software situation?
> 
> 
>> because these are a waste of time
> 
> How do you value compliance with coding styles?

Just stop sending these kind of patches, *please*.
Linux has tons of issues, fixes for real problems are very welcome.
But coding style bike shedding is just a waste of time.

Thanks,
//richard

Re: md/raid1: Improve another size determination in setup_conf()

[Richard Weinberger]

On 07.10.2016 10:53, SF Markus Elfring wrote:
 Replace the specification of a data structure by a pointer dereference
 as the parameter for the operator "sizeof" to make the corresponding size
 determination a bit safer.
>>>
>>> Isn't this pure matter of taste?
>>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>>> developers prefer sizeof(struct foo) because you can determine the type
>>> at first sight and makes review more easy.
>>
>> sizeof(*ptr) is more future proof and normally more obvious and easier
>> to review.
> 
> Is it interesting to see how different the software development opinions
> can be for such an implementation detail?
> 
> 
>> That said, I've tried to tell Markus to only send bugfix patches
> 
> Can any deviations from the Linux coding style become "bugs" also in
> your view of the software situation?
> 
> 
>> because these are a waste of time
> 
> How do you value compliance with coding styles?

Just stop sending these kind of patches, *please*.
Linux has tons of issues, fixes for real problems are very welcome.
But coding style bike shedding is just a waste of time.

Thanks,
//richard

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>>> Replace the specification of a data structure by a pointer dereference
>>> as the parameter for the operator "sizeof" to make the corresponding size
>>> determination a bit safer.
>>
>> Isn't this pure matter of taste?
>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>> developers prefer sizeof(struct foo) because you can determine the type
>> at first sight and makes review more easy.
> 
> sizeof(*ptr) is more future proof and normally more obvious and easier
> to review.

Is it interesting to see how different the software development opinions
can be for such an implementation detail?


> That said, I've tried to tell Markus to only send bugfix patches

Can any deviations from the Linux coding style become "bugs" also in
your view of the software situation?


> because these are a waste of time

How do you value compliance with coding styles?


> and regularly introduce bugs.

Really?

Would you like to discuss concrete incidents any further?

Regards,
Markus

Re: md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

>>> Replace the specification of a data structure by a pointer dereference
>>> as the parameter for the operator "sizeof" to make the corresponding size
>>> determination a bit safer.
>>
>> Isn't this pure matter of taste?
>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>> developers prefer sizeof(struct foo) because you can determine the type
>> at first sight and makes review more easy.
> 
> sizeof(*ptr) is more future proof and normally more obvious and easier
> to review.

Is it interesting to see how different the software development opinions
can be for such an implementation detail?


> That said, I've tried to tell Markus to only send bugfix patches

Can any deviations from the Linux coding style become "bugs" also in
your view of the software situation?


> because these are a waste of time

How do you value compliance with coding styles?


> and regularly introduce bugs.

Really?

Would you like to discuss concrete incidents any further?

Regards,
Markus

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Richard Weinberger]

On 07.10.2016 09:53, Dan Carpenter wrote:
> On Thu, Oct 06, 2016 at 11:29:20AM +0200, Richard Weinberger wrote:
>> On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
>>  wrote:
>>> From: Markus Elfring 
>>> Date: Tue, 4 Oct 2016 21:46:18 +0200
>>>
>>> Replace the specification of a data structure by a pointer dereference
>>> as the parameter for the operator "sizeof" to make the corresponding size
>>> determination a bit safer.
>>
>> Isn't this pure matter of taste?
>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>> developers prefer sizeof(struct foo) because you can determine the type
>> at first sight and makes review more easy.
> 
> sizeof(*ptr) is more future proof and normally more obvious and easier
> to review.

Also a matter of taste.
See http://yarchive.net/comp/linux/struct_init.html

Thanks,
//richard

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Richard Weinberger]

On 07.10.2016 09:53, Dan Carpenter wrote:
> On Thu, Oct 06, 2016 at 11:29:20AM +0200, Richard Weinberger wrote:
>> On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
>>  wrote:
>>> From: Markus Elfring 
>>> Date: Tue, 4 Oct 2016 21:46:18 +0200
>>>
>>> Replace the specification of a data structure by a pointer dereference
>>> as the parameter for the operator "sizeof" to make the corresponding size
>>> determination a bit safer.
>>
>> Isn't this pure matter of taste?
>> Some developers prefer sizeof(*ptr) because it is easier to type, other
>> developers prefer sizeof(struct foo) because you can determine the type
>> at first sight and makes review more easy.
> 
> sizeof(*ptr) is more future proof and normally more obvious and easier
> to review.

Also a matter of taste.
See http://yarchive.net/comp/linux/struct_init.html

Thanks,
//richard

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Dan Carpenter]

On Thu, Oct 06, 2016 at 11:29:20AM +0200, Richard Weinberger wrote:
> On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
>  wrote:
> > From: Markus Elfring 
> > Date: Tue, 4 Oct 2016 21:46:18 +0200
> >
> > Replace the specification of a data structure by a pointer dereference
> > as the parameter for the operator "sizeof" to make the corresponding size
> > determination a bit safer.
> 
> Isn't this pure matter of taste?
> Some developers prefer sizeof(*ptr) because it is easier to type, other
> developers prefer sizeof(struct foo) because you can determine the type
> at first sight and makes review more easy.

sizeof(*ptr) is more future proof and normally more obvious and easier
to review.  That said, I've tried to tell Markus to only send bugfix
patches because these are a waste of time and regularly introduce bugs.

regards,
dan carpenter

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Dan Carpenter]

On Thu, Oct 06, 2016 at 11:29:20AM +0200, Richard Weinberger wrote:
> On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
>  wrote:
> > From: Markus Elfring 
> > Date: Tue, 4 Oct 2016 21:46:18 +0200
> >
> > Replace the specification of a data structure by a pointer dereference
> > as the parameter for the operator "sizeof" to make the corresponding size
> > determination a bit safer.
> 
> Isn't this pure matter of taste?
> Some developers prefer sizeof(*ptr) because it is easier to type, other
> developers prefer sizeof(struct foo) because you can determine the type
> at first sight and makes review more easy.

sizeof(*ptr) is more future proof and normally more obvious and easier
to review.  That said, I've tried to tell Markus to only send bugfix
patches because these are a waste of time and regularly introduce bugs.

regards,
dan carpenter

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Richard Weinberger]

On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Tue, 4 Oct 2016 21:46:18 +0200
>
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer.

Isn't this pure matter of taste?
Some developers prefer sizeof(*ptr) because it is easier to type, other
developers prefer sizeof(struct foo) because you can determine the type
at first sight and makes review more easy.

-- 
Thanks,
//richard

Re: [PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[Richard Weinberger]

On Thu, Oct 6, 2016 at 11:22 AM, SF Markus Elfring
 wrote:
> From: Markus Elfring 
> Date: Tue, 4 Oct 2016 21:46:18 +0200
>
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer.

Isn't this pure matter of taste?
Some developers prefer sizeof(*ptr) because it is easier to type, other
developers prefer sizeof(struct foo) because you can determine the type
at first sight and makes review more easy.

-- 
Thanks,
//richard

[PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 4 Oct 2016 21:46:18 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

Signed-off-by: Markus Elfring 
---
 drivers/md/raid1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index ec6aafb..5e1a427 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2776,7 +2776,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
struct md_rdev *rdev;
int err;
 
-   conf = kzalloc(sizeof(struct r1conf), GFP_KERNEL);
+   conf = kzalloc(sizeof(*conf), GFP_KERNEL);
if (!conf)
return ERR_PTR(-ENOMEM);
 
-- 
2.10.1

[PATCH 24/54] md/raid1: Improve another size determination in setup_conf()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 4 Oct 2016 21:46:18 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer.

Signed-off-by: Markus Elfring 
---
 drivers/md/raid1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index ec6aafb..5e1a427 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2776,7 +2776,7 @@ static struct r1conf *setup_conf(struct mddev *mddev)
struct md_rdev *rdev;
int err;
 
-   conf = kzalloc(sizeof(struct r1conf), GFP_KERNEL);
+   conf = kzalloc(sizeof(*conf), GFP_KERNEL);
if (!conf)
return ERR_PTR(-ENOMEM);
 
-- 
2.10.1