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

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

[Nishka Dasgupta]

---
 drivers/staging/ks7010/ks_wlan_net.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index e48c557..69cd8347 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -201,6 +201,8 @@ static int ks_wlan_set_freq(struct net_device *dev,
 {
 	struct ks_wlan_private *priv = netdev_priv(dev);
 	int channel;
+	int a2412 = 2.412e8;
+	int b2487 = 2.487e8;
 
 	if (priv->sleep_mode == SLP_SLEEP)
 		return -EPERM;
@@ -208,7 +210,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
 	/* for SLEEP MODE */
 	/* If setting by frequency, convert to a channel */
 	if ((fwrq->e == 1) &&
-	    (fwrq->m >= (int)2.412e8) && (fwrq->m <= (int)2.487e8)) {
+	    (fwrq->m >= a2412) && (fwrq->m <= b2487)) {
 		int f = fwrq->m / 100000;
 		int c = 0;
 
-- 
1.9.1

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

[Gargi Sharma]

Hi Nishka,

Thanks for your patch. A few things you would want to take care of:
1) Subject line: Has two [Patch]. There should be only one. Also, if
you see the other patches in the mailing list, you can see the subject
line should be {driver}: {filename}.

On Mon, Feb 26, 2018 at 6:18 PM, Nishka Dasgupta
<nishka.dasgupta_ug18@ashoka.edu.in> wrote:

2) Here you would want to mention what changes you made and why you
made them. Also mention if you used any tools. The signed off by would
also go here and not the subject line.
3) You would also want to add the maintainer of this driver to the
patch you submit.

> ---
>  drivers/staging/ks7010/ks_wlan_net.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
> index e48c557..69cd8347 100644
> --- a/drivers/staging/ks7010/ks_wlan_net.c
> +++ b/drivers/staging/ks7010/ks_wlan_net.c
> @@ -201,6 +201,8 @@ static int ks_wlan_set_freq(struct net_device *dev,
>  {
>         struct ks_wlan_private *priv = netdev_priv(dev);
>         int channel;
> +       int a2412 = 2.412e8;
> +       int b2487 = 2.487e8;
>
>         if (priv->sleep_mode == SLP_SLEEP)
>                 return -EPERM;
> @@ -208,7 +210,7 @@ static int ks_wlan_set_freq(struct net_device *dev,
>         /* for SLEEP MODE */
>         /* If setting by frequency, convert to a channel */
>         if ((fwrq->e == 1) &&
> -           (fwrq->m >= (int)2.412e8) && (fwrq->m <= (int)2.487e8)) {
> +           (fwrq->m >= a2412) && (fwrq->m <= b2487)) {

4) I don't think this is right. Why do you have an a and a b prepended?
5) Lastly did you compile the kernel before sending in your changes?
If not, please do.

Thanks!
Gargi

>                 int f = fwrq->m / 100000;
>                 int c = 0;
>
> --
> 1.9.1
>
> --
> 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/1519669095-31351-1-git-send-email-nishka.dasgupta_ug18%40ashoka.edu.in.
> For more options, visit https://groups.google.com/d/optout.

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>

[Nishka Dasgupta]

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?

Thanking you,
Nishka Dasgupta

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.
>

Re:

[Nishka Dasgupta]

Hi,
Thank you for the clarification!
Regards,
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.
>