All of lore.kernel.org
 help / color / mirror / Atom feed
From: Holger Macht <holger@homac.de>
To: Jeff Garzik <jeff@garzik.org>
Cc: Hugh Dickins <hughd@google.com>, Hillf Danton <dhillf@gmail.com>,
	Matthew Garrett <mjg@redhat.com>,
	Jeff Garzik <jgarzik@redhat.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: linux-next: dock_link_device is oopsy
Date: Tue, 21 Feb 2012 23:30:25 +0100	[thread overview]
Message-ID: <20120221223025.GA12989@homac.suse.de> (raw)
In-Reply-To: <4F441998.5080600@garzik.org>

On Tue 21. Feb - 17:24:24, Jeff Garzik wrote:
> On 02/18/2012 02:57 PM, Holger Macht wrote:
> >On Sa 18. Feb - 10:46:04, Hugh Dickins wrote:
> >>On Sat, 18 Feb 2012, Holger Macht wrote:
> >>>How about that one?
> >>
> >>It's more broken than that.  Here's my attempt.  It boots on the
> >>systems with dock_station_count 0, and it boots on my laptop with
> >>dock_station_count 2; but I don't actually have any docking station,
> >>so it still doesn't test very much (dock is 0 after the loop).
> >
> >Well, there doesn't have to actually exist a physical dock station (or
> >bay device) for dock_station_count to be>  0. It just tells that the
> >ACPI objects are present and thus the system is capable of it.
> >
> >So does this function actually also break on your laptop and you're
> >getting the oops there, too?
> >
> >>I have no idea if what goes on in the loop is correct, but it looks
> >>to me as if (as predicted) there's further breakage, that it would
> >>have been writing beyond the end of what it allocated if I did have
> >>a docking station.
> >>
> >>Hugh
> >>
> >>[PATCH] dock: fix bootup oops and other dock_link breakage
> >>
> >>dock_link_device() and dock_unlink_device() should bail out early
> >>to avoid oops on zero-length kmalloc() when dock_station_count is 0.
> >>
> >>But isn't there an off-by-one in that kmalloc() length anyway?
> >>An extra NULL appended at the end suggests so.
> >>
> >>Rework the ordering with gotos on failure to fix several issues.
> >>
> >>And presumably dock_unlink_device() should be presenting the same
> >>interface as dock_link_device(), with NULL returned when none found.
> >>
> >>Signed-off-by: Hugh Dickins<hughd@google.com>
> >
> >Fine with me.
> 
> So, just to be clear, the preferred patch is Hugh's, and I should
> drop your earlier proposed fix found in this thread?

Correct.

> And what about that warning?

You mean the fix for the compile error when compiling with
CONFIG_ACPI_DOCK=n? Here it is again:

[PATCH] acpi: Fix compiler error when setting CONFIG_ACPI_DOCK=n

When compiling with CONFIG_ACPI_DOCK=n,
is_registered_hotplug_dock_device() needs to be defined

Signed-off-by: Holger Macht <holger@homac.de>
---
 include/acpi/acpi_drivers.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 3c4e381..3319574 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -155,6 +155,10 @@ static inline int register_hotplug_dock_device(acpi_handle handle,
 static inline void unregister_hotplug_dock_device(acpi_handle handle)
 {
 }
+static inline int is_registered_hotplug_dock_device(const struct acpi_dock_ops *ops)
+{
+	return 0;
+}
 static inline struct device **dock_link_device(acpi_handle handle)
 {
 	return NULL;
-- 
1.7.7


  reply	other threads:[~2012-02-21 22:30 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-17 21:46 linux-next: dock_link_device is oopsy Hugh Dickins
2012-02-17 22:29 ` Holger Macht
2012-02-17 22:42   ` Hugh Dickins
2012-02-17 23:01     ` Holger Macht
2012-02-17 23:49       ` Hugh Dickins
2012-02-18 11:14         ` Holger Macht
2012-02-18 13:05           ` Hillf Danton
2012-02-18 13:26             ` Holger Macht
2012-02-18 13:37               ` Hillf Danton
2012-02-18 14:04                 ` Holger Macht
2012-02-18 14:35                   ` Hillf Danton
2012-02-18 18:46                   ` Hugh Dickins
2012-02-18 19:57                     ` Holger Macht
2012-02-18 21:03                       ` Hugh Dickins
2012-02-18 21:50                         ` Holger Macht
2012-02-21 22:24                       ` Jeff Garzik
2012-02-21 22:30                         ` Holger Macht [this message]
2012-02-18  7:52       ` Hugh Dickins

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=20120221223025.GA12989@homac.suse.de \
    --to=holger@homac.de \
    --cc=akpm@linux-foundation.org \
    --cc=dhillf@gmail.com \
    --cc=hughd@google.com \
    --cc=jeff@garzik.org \
    --cc=jgarzik@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg@redhat.com \
    --cc=sfr@canb.auug.org.au \
    /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.