kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kernel-janitors@vger.kernel.org
Subject: [bug report] net: aquantia: add basic ptp_clock callbacks
Date: Mon, 28 Oct 2019 11:31:58 +0000	[thread overview]
Message-ID: <20191028113158.GA32279@mwanda> (raw)

Hello Egor Pomozov,

The patch 910479a9f793: "net: aquantia: add basic ptp_clock
callbacks" from Oct 22, 2019, leads to the following static checker
warning:

	drivers/net/ethernet/aquantia/atlantic/aq_ptp.c:1208 aq_ptp_init()
	warn: 'clock' is an error pointer or valid

drivers/net/ethernet/aquantia/atlantic/aq_ptp.c
  1203          spin_lock_init(&aq_ptp->ptp_ring_lock);
  1204  
  1205          aq_ptp->ptp_info = aq_ptp_clock;
  1206          aq_ptp_gpio_init(&aq_ptp->ptp_info, &mbox.info);
  1207          clock = ptp_clock_register(&aq_ptp->ptp_info, &aq_nic->ndev->dev);
  1208          if (!clock || IS_ERR(clock)) {
  1209                  netdev_err(aq_nic->ndev, "ptp_clock_register failed\n");
  1210                  err = PTR_ERR(clock);
  1211                  goto err_exit;

This is a false positive in Smatch but the code is still problematic.

The issue is that ptp_clock_register() returns error pointers if there
is an error and it returns NULL if the clock is disabled in the config.
If "clock" is NULL then this code returns PTR_ERR(NULL) which is
success but so that's a bug.

The question is, is it really realistic for people to run this hardware
without a ptp clock?  If so then we should allow it instead of erroring
out here when clock is NULL.  If not then we should enforce that in the
Kconfig instead of waiting until runtime.

  1212          }
  1213          aq_ptp->ptp_clock = clock;
  1214          aq_ptp_tx_timeout_init(&aq_ptp->ptp_tx_timeout);
  1215  
  1216          atomic_set(&aq_ptp->offset_egress, 0);
  1217          atomic_set(&aq_ptp->offset_ingress, 0);
  1218  

regards,
dan carpenter

                 reply	other threads:[~2019-10-28 11:31 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20191028113158.GA32279@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=kernel-janitors@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).