public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Alex Chiang <achiang@hp.com>
To: Rakib Mullick <rakib.mullick@gmail.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>,
	linux-pci@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] pci: Make pci_dev struct point to NULL.
Date: Fri, 4 Dec 2009 22:28:02 -0700	[thread overview]
Message-ID: <20091205052802.GD10777@ldl.fc.hp.com> (raw)
In-Reply-To: <b9df5fa10912042118p733416ffwe21b79b1f8246f2f@mail.gmail.com>

* Rakib Mullick <rakib.mullick@gmail.com>:
> 
> Since *pdev might be uninitialized. But if we are sure that it
> is not uninitialized then it is okay.
> 
> And yes - althrough we weren't warned by the compiler.

Let's start over.

This is the function:

 965 static int __ref enable_device(struct acpiphp_slot *slot)
 966 {
 967         struct pci_dev *dev;

Your patch makes this change:
             struct pci_dev *dev = NULL;

 968         struct pci_bus *bus = slot->bridge->pci_bus;
 969         struct list_head *l;
 970         struct acpiphp_func *func;
 971         int retval = 0;
 972         int num, max, pass;
 973         acpi_status status;
 974 
 975         if (slot->flags & SLOT_ENABLED)
 976                 goto err_exit;

Here, if the slot is already enabled, we goto the err_exit label
(below). We haven't touched 'dev' yet.

Otherwise, we call pci_get_slot().

 977 
 978         /* sanity check: dev should be NULL when hot-plugged in */
 979         dev = pci_get_slot(bus, PCI_DEVFN(slot->device, 0));
 980         if (dev) {
 981                 /* This case shouldn't happen */
 982                 err("pci_dev structure already exists.\n");
 983                 pci_dev_put(dev);
 984                 retval = -1;
 985                 goto err_exit;
 986         }

If pci_get_slot() finds the devfn, it returns the pointer to the
pdev, puts it into 'dev' and we return early.

If it cannot find the devfn, then we put NULL into dev and
continue with the rest of the function.

1044  err_exit:
1045         return retval;
1046 }


At no point that I can tell do we ever access an uninitialized
'dev'.

Please explain to me one more time what you think you are fixing.

Thanks,
/ac

  reply	other threads:[~2009-12-05  5:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-04 16:35 [PATCH] pci: Make pci_dev struct point to NULL Rakib Mullick
2009-12-04 20:27 ` Alex Chiang
2009-12-05  1:53   ` Rakib Mullick
2009-12-05  4:36     ` Alex Chiang
2009-12-05  4:37       ` Alex Chiang
2009-12-05  5:18       ` Rakib Mullick
2009-12-05  5:28         ` Alex Chiang [this message]
2009-12-05  5:47           ` Rakib Mullick

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=20091205052802.GD10777@ldl.fc.hp.com \
    --to=achiang@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=jbarnes@virtuousgeek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rakib.mullick@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox