[PATCH v2] staging: vc04_services: bcm2835-camera: Add blank line

[PATCH v2] staging: vc04_services: bcm2835-camera: Add blank line

[Nishka Dasgupta]

Add blank line after variable declaration. Issue found with checkpatch.

Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
---
Changes in v2:
 - Format commit message.

 drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index e3460b7..c90754f 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -1979,6 +1979,7 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)
 	static VCHI_CONNECTION_T *vchi_connection;
 
 	static VCHI_INSTANCE_T vchi_instance;
+
 	SERVICE_CREATION_T params = {
 		.version		= VCHI_VERSION_EX(VC_MMAL_VER, VC_MMAL_MIN_VER),
 		.service_id		= VC_MMAL_SERVER_NAME,
-- 
1.9.1

Re: [PATCH v2] staging: vc04_services: bcm2835-camera: Add blank line

[Greg KH]

On Thu, Mar 01, 2018 at 11:56:13PM +0530, Nishka Dasgupta wrote:
> Add blank line after variable declaration. Issue found with checkpatch.
> 
> Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>
> ---
> Changes in v2:
>  - Format commit message.
> 
>  drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> index e3460b7..c90754f 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
> @@ -1979,6 +1979,7 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)
>  	static VCHI_CONNECTION_T *vchi_connection;
>  
>  	static VCHI_INSTANCE_T vchi_instance;
> +
>  	SERVICE_CREATION_T params = {
>  		.version		= VCHI_VERSION_EX(VC_MMAL_VER, VC_MMAL_MIN_VER),
>  		.service_id		= VC_MMAL_SERVER_NAME,

Checkpatch is wrong here, don't you think?  Are you sure this is
actually doing what you think it is?

thanks,

greg k-h

Re: [PATCH v2] staging: vc04_services: bcm2835-camera: Add blank line

[Nishka Dasgupta]

Checkpatch suggested two warnings for this file in consecutive lines: "static VCHI_CONNECTION_T *vchi_connection;" and "static VCHI_INSTANCE_T vchi_instance;". Both warnings said to add a blank line after the declaration. If checkpatch was wrong, is it okay if I submit a version 2 with a blank line only after "vchi_instance" and not below "*vchi_connection" (effectively undoing one of my commits)?

Thanking you,
Nishka Dasgupta

Re: [PATCH v2] staging: vc04_services: bcm2835-camera: Add blank line

[Greg KH]

On Fri, Mar 02, 2018 at 12:45:48AM +0530, Nishka Dasgupta wrote:
> Checkpatch suggested two warnings for this file in consecutive lines: "static VCHI_CONNECTION_T *vchi_connection;" and "static VCHI_INSTANCE_T vchi_instance;". Both warnings said to add a blank line after the declaration. If checkpatch was wrong, is it okay if I submit a version 2 with a blank line only after "vchi_instance" and not below "*vchi_connection" (effectively undoing one of my commits)?


First off, I have no context as to what you are responding to here,
please always quote the email you are responding to properly.  Reviewers
deal with hundreds of emails a day, and not having context for what you
say doesn't really work :(

Also, properly wrap your email lines at 72 columns, your email client
should do this for you, right?

Please respond to the email with the correct context and I will be glad
to respond.  Right now, I have no idea what you are referring to.

thanks,

greg k-h

Re:

[Nishka Dasgupta]

This is with reference to your last email pointing out that you have no
context for what I am responding to. Unfortunately, I have been unable
to get mutt to load my inbox, so I could not and cannot quote text
directly. I have done my best to reproduce the conversation below in a
coherent fashion; nonetheless, I apologise for any persisting lack of
clarity.

An hour ago I submitted the following commit with respect to
drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c: [PATCH v2]
staging: vc04_services: bcm2835-camera: Add blank line

Your reply: Checkpatch is wrong here, don't you think? Are you sure this
is actually doing what you think it is?

My reply: Checkpatch suggested two warnings for this file in consecutive
lines: "static VCHI_CONNECTION_T *vchi_connection;" and "static
VCHI_INSTANCE_T vchi_instance;". Both warnings said to add a blank line
after the declaration. If checkpatch was wrong, is it okay if I submit a
version 2 with a blank line only after "vchi_instance" and not below
"*vchi_connection" (effectively undoing one of my commits)?

Your response: First off, I have no context as to what you are
responding to here, please always quote the email you are responding to
properly.  Reviewers deal with hundreds of emails a day, and not having
context for what you say doesn't really work :( Also, properly wrap your
email lines at 72 columns, your email client should do this for you,
right?  Please respond to the email with the correct context and I will
be glad to respond.  Right now, I have no idea what you are referring
to.  

(I am sorry for the suboptimal formatting. I think I managed to linewrap
this email properly, however.) My question remains: may I submit a
revised commit that effectively undoes the first "add blank line"
commit, i.e. the one that added a line between "static VCHI_CONNECTION_T
*vchi_connection" and "static VCHI_INSTANCE_T vchi_instance"?  Thank you
for your time, and apologies again for the confusion.

Regards, 
Nishka Dasgupta

Re: [Outreachy kernel] Re:

[Julia Lawall]

On Fri, 2 Mar 2018, Nishka Dasgupta wrote:

> This is with reference to your last email pointing out that you have no
> context for what I am responding to. Unfortunately, I have been unable
> to get mutt to load my inbox, so I could not and cannot quote text
> directly. I have done my best to reproduce the conversation below in a
> coherent fashion; nonetheless, I apologise for any persisting lack of
> clarity.
>
> An hour ago I submitted the following commit with respect to
> drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c: [PATCH v2]
> staging: vc04_services: bcm2835-camera: Add blank line
>
> Your reply: Checkpatch is wrong here, don't you think? Are you sure this
> is actually doing what you think it is?
>
> My reply: Checkpatch suggested two warnings for this file in consecutive
> lines: "static VCHI_CONNECTION_T *vchi_connection;" and "static
> VCHI_INSTANCE_T vchi_instance;". Both warnings said to add a blank line
> after the declaration. If checkpatch was wrong, is it okay if I submit a
> version 2 with a blank line only after "vchi_instance" and not below
> "*vchi_connection" (effectively undoing one of my commits)?
>
> Your response: First off, I have no context as to what you are
> responding to here, please always quote the email you are responding to
> properly.  Reviewers deal with hundreds of emails a day, and not having
> context for what you say doesn't really work :( Also, properly wrap your
> email lines at 72 columns, your email client should do this for you,
> right?  Please respond to the email with the correct context and I will
> be glad to respond.  Right now, I have no idea what you are referring
> to.
>
> (I am sorry for the suboptimal formatting. I think I managed to linewrap
> this email properly, however.) My question remains: may I submit a
> revised commit that effectively undoes the first "add blank line"
> commit, i.e. the one that added a line between "static VCHI_CONNECTION_T
> *vchi_connection" and "static VCHI_INSTANCE_T vchi_instance"?  Thank you
> for your time, and apologies again for the confusion.

You don't submit patches to fix the incorrect changes you have proposed
and that have not been integrated.  Just start with the original staging
tree and send patches against that.

You can use git rebase to remove a previous commit in your tree (read the
manual to find the proper options).  Or if you don't care about your tree,
you can remove it and start over.

julia


>
> Regards,
> Nishka Dasgupta
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1519935610-27428-1-git-send-email-nishka.dasgupta_ug18%40ashoka.edu.in.
> For more options, visit https://groups.google.com/d/optout.
>

Re:

[Greg KH]

On Fri, Mar 02, 2018 at 01:50:10AM +0530, Nishka Dasgupta wrote:
> This is with reference to your last email pointing out that you have no
> context for what I am responding to. Unfortunately, I have been unable
> to get mutt to load my inbox, so I could not and cannot quote text
> directly. I have done my best to reproduce the conversation below in a
> coherent fashion; nonetheless, I apologise for any persisting lack of
> clarity.

If mutt can't read your inbox, how are you reading it at all? :)

> An hour ago I submitted the following commit with respect to
> drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c: [PATCH v2]
> staging: vc04_services: bcm2835-camera: Add blank line
> 
> Your reply: Checkpatch is wrong here, don't you think? Are you sure this
> is actually doing what you think it is?
> 
> My reply: Checkpatch suggested two warnings for this file in consecutive
> lines: "static VCHI_CONNECTION_T *vchi_connection;" and "static
> VCHI_INSTANCE_T vchi_instance;". Both warnings said to add a blank line
> after the declaration. If checkpatch was wrong, is it okay if I submit a
> version 2 with a blank line only after "vchi_instance" and not below
> "*vchi_connection" (effectively undoing one of my commits)?

Look at the code, and see what checkpatch is telling you and see if that
actually matches with what the code shows.  Then make the change that
you know is correct based on your knowledge of C, and how the code
should look.  Hint, checkpatch is wrong here, but the code is also wrong
as-is.

thanks,

greg k-h

Re:

[Nishka Dasgupta]

This is with reference to the Patch I submitted for
staging/vc04_services/bcm2835-camera: Add blank line. (Since the last
message on that thread, I have managed to configure mutt, but several of
the more recent emails, however, failed to load in mutt, which is why I
am still not replying inline. I should be able to respond to all future
emails inline, however.)

The last email on the thread ended:
> Look at the code, and see what checkpatch is telling you and see if
> that actually matches with what the code shows. Then make the change
> that you know is correct based on your knowledge of C, and how the
> code should look. Hint, checkpatch is wrong here, but the code is also
> wrong as-is.
>
> thanks, greg k-h

I am afraid I still have not been able to locate the error. Is it a
problem of too many or too few asterisks in the line "static
VCHI_CONNECTION_T *vchi_connection"?

Thank you for your help!

Regards, 
Nishka Dasgupta

Re:

[Greg KH]

On Thu, Mar 08, 2018 at 11:53:22PM +0530, Nishka Dasgupta wrote:
> The last email on the thread ended:
> > Look at the code, and see what checkpatch is telling you and see if
> > that actually matches with what the code shows. Then make the change
> > that you know is correct based on your knowledge of C, and how the
> > code should look. Hint, checkpatch is wrong here, but the code is also
> > wrong as-is.
> >
> > thanks, greg k-h
> 
> I am afraid I still have not been able to locate the error. Is it a
> problem of too many or too few asterisks in the line "static
> VCHI_CONNECTION_T *vchi_connection"?

I have no context at all here, to know what you are talking about, and
your emails are long-gone from my queue, sorry.

That being said, asterisks mean special things in the C language.  Be
sure you understand what they are, how they work, and when they are
needed.  Perhaps some more C language experience is needed here before
you work on the kernel?

thanks,

greg k-h

Re: [Outreachy kernel] Re: Re: [PATCH] h [Patch] Fixed unnecessary typecasting to in. Error found with checkpatch. Signed-off-by: Nishka Dasgupta <nishka.dasgupta_ug18@ashoka.edu.in>

[Julia Lawall]

On Tue, 27 Feb 2018, Nishka Dasgupta wrote:

> Hi,
> Thank you for pointing out the formatting errors. I have deleted the unnecessary [Patch] and added the driver information.
> I could not, however, work out how to separate the subject line from the body of the text.
> I sent the commit to the maintainer of the patch in a separate email as I was (and am) not comfortable with git send-email, though I am doing my best to learn. I have sent him the revised patch, however.
> I don't know why a and b are prepended, unless you are referring to the variables that I added, in which case I would like to clarify that I did not want to use names as generic and vague as a and b for the new variables, so I appended part of their respective values after the initial character for ease of understanding. Should I have used more descriptive variable names?
> Finally, I did compile the this particular driver (make path/ as on the website) and an output file was generated for the modified file, so I took that to mean it was working as it should. Was I wrong?

I think that the code is actually better as is.  Normally, there are not
floating point numbers in kernel code, so it is nice to see the int cast,
and to see what the numbers are being used for.  Checkpatch is not always
correct...

julia

Re:

[Nishka Dasgupta]

Hi,
Weren't the original values already integers because of the "e8" appended?
If this was an unnecessary patch, I will try to do better next time.
Thanking you,
Nishka Dasgupta

Re: [Outreachy kernel] Re:

[Julia Lawall]

On Tue, 27 Feb 2018, Nishka Dasgupta wrote:

> Hi,
> Weren't the original values already integers because of the "e8" appended?
> If this was an unnecessary patch, I will try to do better next time.

I think that they remain floats.

julia

> Thanking you,
> Nishka Dasgupta
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1519739588-3783-1-git-send-email-nishka.dasgupta_ug18%40ashoka.edu.in.
> For more options, visit https://groups.google.com/d/optout.
>