Messages in this thread | | | Date | Mon, 9 Apr 2012 07:05:31 -0700 | Subject | Re: [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver | From | Olof Johansson <> |
| |
On Mon, Apr 9, 2012 at 1:46 AM, Jingoo Han <jg1.han@samsung.com> wrote: >> -----Original Message----- >> From: Olof Johansson [mailto:olof@lixom.net] >> Sent: Monday, April 09, 2012 1:16 PM >> >> I'm not 100% sure if the fix for 'adjust_request' is correct, since >> it's uncertain what the original intent was. But it's so clearly an >> uninitialized pointer dereference that my resolution seems to make sense. >> >> drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train': >> drivers/video/exynos/exynos_dp_core.c:521:21: warning: 'adjust_request' may be used uninitialized in >> this function [-Wuninitialized] >> drivers/video/exynos/exynos_dp_core.c:481:6: note: 'adjust_request' was declared here >> drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function >> [-Wuninitialized] >> drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here >> >> Signed-off-by: Olof Johansson <olof@lixom.net> >> --- >> drivers/video/exynos/exynos_dp_core.c | 23 +++++++++-------------- >> 1 file changed, 9 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c >> index 2a4481c..8973e18 100644 >> --- a/drivers/video/exynos/exynos_dp_core.c >> +++ b/drivers/video/exynos/exynos_dp_core.c >> @@ -392,24 +392,19 @@ static unsigned int exynos_dp_get_lane_link_training( >> struct exynos_dp_device *dp, >> int lane) >> { >> - u32 reg; >> - >> switch (lane) { >> case 0: >> - reg = exynos_dp_get_lane0_link_training(dp); >> - break; >> + return exynos_dp_get_lane0_link_training(dp); >> case 1: >> - reg = exynos_dp_get_lane1_link_training(dp); >> - break; >> + return exynos_dp_get_lane1_link_training(dp); >> case 2: >> - reg = exynos_dp_get_lane2_link_training(dp); >> - break; >> + return exynos_dp_get_lane2_link_training(dp); >> case 3: >> - reg = exynos_dp_get_lane3_link_training(dp); >> - break; >> + return exynos_dp_get_lane3_link_training(dp); >> } > > I don't like multi return.
In a small helper function like this there's nothing wrong with it. Larger functions? Sure.
Adding a default in the switch didn't seem like an improvement to me. But I'll leave it up to you.
>> >> - return reg; >> + WARN_ON(1); >> + return 0; >> } >> >> static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp) >> @@ -489,13 +484,13 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp) >> 6, link_status); >> lane_count = dp->link_train.lane_count; >> >> + adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1 >> + - DPCD_ADDR_LANE0_1_STATUS); >> + > > It makes the problem. adjust_request will be different. > > OK, I understand what you want to do. > I will send the version 2 patch which is simpler. > > Thank you for sending the patch.
Ok, thanks. As I said, I'm not sure what your intent with the second (else) code path was.
-Olof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |