From: Federico Vaga <federico.vaga@cern.ch>
To: Alan Tull <atull@kernel.org>
Cc: <linux-fpga@vger.kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>
Subject: fpga: fpga_mgr_free usage
Date: Wed, 11 Jul 2018 14:38:29 +0200 [thread overview]
Message-ID: <5553262.evi6GU8epL@pcbe13614> (raw)
Hi Alan,
I have another point that I would like to discuss. It is about the
usage of 'fpga_mgr_free()' which does not look like consistent.
This function, according to the current implementation, can be used by
an FPGA manager user and it is used by the FPGA manager itself on
device release. This means that the user can only use this function if
fpga_mgr_register() fails (to clean up), otherwise the user must NOT
use this function, otherwise we most likely get an oops (NULL or
invalid pointer). The example here is correct, this is what we should
do:
https://www.kernel.org/doc/html/latest/driver-api/fpga/fpga-mgr.html
But I suggest to document it better or prevent this type of mistakes
from happening. Following a couple of proposals
------
1.
Document it better. This is easy, in the fpga_mgr_free() kernel-doc
comment we explain that the use of this function must be limited to
clean up the memory on a registration failure. If an FPGA manager has
been successfully registered then it will be freed by the framework
itself.
But still, this does not prevent an oops from happening.
------
2.
Remove the fpga_mgr_free() from fpga_mgr_dev_release() and ask the
user to free the manager when necessary.
This makes the usage consistent: the user creates and destroy its own
objects. This is also consistent with our other discussion where we
said, among the other things, that the module that uses the FPGA
manager can the owner of the fpga_manager data structure.
------
3.
Not sure how, but perhaps we can be able to understand if we can
safely continue to run fpga_mgr_free() or we should stop.
------
next reply other threads:[~2018-07-11 12:38 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-11 12:38 Federico Vaga [this message]
2018-07-11 15:59 ` fpga: fpga_mgr_free usage Alan Tull
2018-07-11 18:25 ` Moritz Fischer
2018-07-25 16:33 ` Alan Tull
2018-07-26 7:27 ` Federico Vaga
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=5553262.evi6GU8epL@pcbe13614 \
--to=federico.vaga@cern.ch \
--cc=atull@kernel.org \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).