From: Ojaswin Mujoo <ojaswin98@gmail.com>
To: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
nsaenz@kernel.org, gregkh@linuxfoundation.org, arnd@arndb.de,
phil@raspberrypi.com, bcm-kernel-feedback-list@broadcom.com,
linux-arm-kernel@lists.infradead.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: staging: vc04_services: Need suggestions on trying to fix sparse warning in vchiq_arm.c
Date: Sat, 5 Jun 2021 12:53:31 +0530 [thread overview]
Message-ID: <20210605072249.GA5967@ojas> (raw)
In-Reply-To: <9ba341f7-17fc-980d-a7a0-2293c75dcf92@i2se.com>
On Fri, Jun 04, 2021 at 08:13:06AM +0200, Stefan Wahren wrote:
> Hi Ojaswin,
>
> Am 02.06.21 um 16:50 schrieb Ojaswin Mujoo:
> > On Tue, Jun 01, 2021 at 11:23:07PM +0300, Dan Carpenter wrote:
> >> The problem is not the Sparse warning, the problem is that this code is
> >> a mess. It used to very clearly buggy and I reported the bug. I think
> >> Arnd found the bug again independently and fixed it.
> >>
> >> A couple weeks ago Al Viro looked at this code. Here is his write up:
> >>
> >> https://www.spinics.net/lists/kernel/msg3952745.html
> >>
> >> It shouldn't take Al Viro dozens of pages of detailed analysis to try
> >> figure out if the code is safe or not. Your idea silences the warning
> >> but would make the code even more subtle and complicated.
> >>
> >> The right thing to do is to re-write the code to be simpler.
> >>
> >> regards,
> >> dan carpenter
> >>
> > Thank you for the prompt reply and the link, it was very insightful. You
> > are right, I was definitely going about this the wrong way and missing
> > the larger picture. I'll spend some time trying to understand this
> > codebase as I think that'd be a good start to understand how stuff works in
> > the kernel (even though some of the things in this driver are anti patterns)
> > and hopefully get some ideas on ways to clean this up.
> >
> > Anyways, thanks again for the help, cheers!
>
> thanks for your interest in cleaning this up. Yes, it's not clear which
> points on the TODO list are the lower hanging fruits. In case you don't
> want to fix checkpatch issues, maybe you can look at points 8, 9, 10, 12
> and 13. Most of them require testing with a Raspberry Pi, but feel free
> to ask if you have problems with it.
>
> Regards
> Stefan
>
Got it, Task 10 (cdev to its own file) seems like a pretty good task to
get started with. I'm planning to buy a Rpi 4 so I think I can run tests
on that.
Thank you so much for the help, I'll get back incase I face any issues down
the line.
Regards,
Ojaswin
> >
> > Ojaswin
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
WARNING: multiple messages have this Message-ID (diff)
From: Ojaswin Mujoo <ojaswin98@gmail.com>
To: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>,
nsaenz@kernel.org, gregkh@linuxfoundation.org, arnd@arndb.de,
phil@raspberrypi.com, bcm-kernel-feedback-list@broadcom.com,
linux-arm-kernel@lists.infradead.org,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: staging: vc04_services: Need suggestions on trying to fix sparse warning in vchiq_arm.c
Date: Sat, 5 Jun 2021 12:53:31 +0530 [thread overview]
Message-ID: <20210605072249.GA5967@ojas> (raw)
In-Reply-To: <9ba341f7-17fc-980d-a7a0-2293c75dcf92@i2se.com>
On Fri, Jun 04, 2021 at 08:13:06AM +0200, Stefan Wahren wrote:
> Hi Ojaswin,
>
> Am 02.06.21 um 16:50 schrieb Ojaswin Mujoo:
> > On Tue, Jun 01, 2021 at 11:23:07PM +0300, Dan Carpenter wrote:
> >> The problem is not the Sparse warning, the problem is that this code is
> >> a mess. It used to very clearly buggy and I reported the bug. I think
> >> Arnd found the bug again independently and fixed it.
> >>
> >> A couple weeks ago Al Viro looked at this code. Here is his write up:
> >>
> >> https://www.spinics.net/lists/kernel/msg3952745.html
> >>
> >> It shouldn't take Al Viro dozens of pages of detailed analysis to try
> >> figure out if the code is safe or not. Your idea silences the warning
> >> but would make the code even more subtle and complicated.
> >>
> >> The right thing to do is to re-write the code to be simpler.
> >>
> >> regards,
> >> dan carpenter
> >>
> > Thank you for the prompt reply and the link, it was very insightful. You
> > are right, I was definitely going about this the wrong way and missing
> > the larger picture. I'll spend some time trying to understand this
> > codebase as I think that'd be a good start to understand how stuff works in
> > the kernel (even though some of the things in this driver are anti patterns)
> > and hopefully get some ideas on ways to clean this up.
> >
> > Anyways, thanks again for the help, cheers!
>
> thanks for your interest in cleaning this up. Yes, it's not clear which
> points on the TODO list are the lower hanging fruits. In case you don't
> want to fix checkpatch issues, maybe you can look at points 8, 9, 10, 12
> and 13. Most of them require testing with a Raspberry Pi, but feel free
> to ask if you have problems with it.
>
> Regards
> Stefan
>
Got it, Task 10 (cdev to its own file) seems like a pretty good task to
get started with. I'm planning to buy a Rpi 4 so I think I can run tests
on that.
Thank you so much for the help, I'll get back incase I face any issues down
the line.
Regards,
Ojaswin
> >
> > Ojaswin
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-06-05 7:23 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-01 20:05 staging: vc04_services: Need suggestions on trying to fix sparse warning in vchiq_arm.c Ojaswin Mujoo
2021-06-01 20:05 ` Ojaswin Mujoo
2021-06-01 20:23 ` Dan Carpenter
2021-06-01 20:23 ` Dan Carpenter
2021-06-02 14:50 ` Ojaswin Mujoo
2021-06-02 14:50 ` Ojaswin Mujoo
2021-06-04 6:13 ` Stefan Wahren
2021-06-04 6:13 ` Stefan Wahren
2021-06-05 7:23 ` Ojaswin Mujoo [this message]
2021-06-05 7:23 ` Ojaswin Mujoo
2021-06-05 8:41 ` Stefan Wahren
2021-06-05 8:41 ` Stefan Wahren
2021-06-06 9:26 ` Ojaswin Mujoo
2021-06-06 9:26 ` Ojaswin Mujoo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210605072249.GA5967@ojas \
--to=ojaswin98@gmail.com \
--cc=arnd@arndb.de \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=dan.carpenter@oracle.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=nsaenz@kernel.org \
--cc=phil@raspberrypi.com \
--cc=stefan.wahren@i2se.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.