Re: [PATCH V2] staging: dgap: introduce dgap_stop()
Hi, 2014-10-09 19:23 GMT+09:00 Dan Carpenter : > On Thu, Oct 09, 2014 at 03:19:03PM +0530, Sudip Mukherjee wrote: >> On Thu, Oct 09, 2014 at 01:40:11PM +0900, Daeseok Youn wrote: >> > The dgap_init_module() need to unwind for cleanup variables properly. >> > Because dgap_init_module() calls dgap_cleanup_module() for freeing >> > variables but this function is possible to free variables >> > which are not allocated. >> > >> > Signed-off-by: Daeseok Youn >> > --- >> > V2: change ulong which is non-standard to "unsigned long". >> I think , Dan, in his review of your v1 , asked you to rearrange the >> functions and get rid of the forward declarations. >> > > I was fine with v1 actually... It's a bug fix, and this is staging and > there are tons of forward declarations already. Eventually we need to > get rid of them but it can be done later. Yes, I will cleanup as your review. thanks. regards, Daeseok Youn. > > regards, > dan carpenter > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] staging: dgap: introduce dgap_stop()
On Thu, Oct 09, 2014 at 03:19:03PM +0530, Sudip Mukherjee wrote: > On Thu, Oct 09, 2014 at 01:40:11PM +0900, Daeseok Youn wrote: > > The dgap_init_module() need to unwind for cleanup variables properly. > > Because dgap_init_module() calls dgap_cleanup_module() for freeing > > variables but this function is possible to free variables > > which are not allocated. > > > > Signed-off-by: Daeseok Youn > > --- > > V2: change ulong which is non-standard to "unsigned long". > I think , Dan, in his review of your v1 , asked you to rearrange the > functions and get rid of the forward declarations. > I was fine with v1 actually... It's a bug fix, and this is staging and there are tons of forward declarations already. Eventually we need to get rid of them but it can be done later. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] staging: dgap: introduce dgap_stop()
On Thu, Oct 09, 2014 at 01:40:11PM +0900, Daeseok Youn wrote:
> The dgap_init_module() need to unwind for cleanup variables properly.
> Because dgap_init_module() calls dgap_cleanup_module() for freeing
> variables but this function is possible to free variables
> which are not allocated.
>
> Signed-off-by: Daeseok Youn
> ---
> V2: change ulong which is non-standard to "unsigned long".
I think , Dan, in his review of your v1 , asked you to rearrange the functions
and get rid of the forward declarations.
thanks
sudip
>
> drivers/staging/dgap/dgap.c | 27 ++-
> 1 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
> index 7c79fe6..290ca3b 100644
> --- a/drivers/staging/dgap/dgap.c
> +++ b/drivers/staging/dgap/dgap.c
> @@ -71,6 +71,7 @@ MODULE_DESCRIPTION("Driver for the Digi International EPCA
> PCI based product lin
> MODULE_SUPPORTED_DEVICE("dgap");
>
> static int dgap_start(void);
> +static void dgap_stop(void);
> static void dgap_init_globals(void);
> static struct board_t *dgap_found_board(struct pci_dev *pdev, int id,
> int boardnum);
> @@ -479,19 +480,20 @@ static int dgap_init_module(void)
>
> rc = pci_register_driver(_driver);
> if (rc)
> - goto err_cleanup;
> + goto err_stop;
>
> rc = dgap_create_driver_sysfiles(_driver);
> if (rc)
> - goto err_cleanup;
> + goto err_unregister;
>
> dgap_driver_state = DRIVER_READY;
>
> return 0;
>
> -err_cleanup:
> -
> - dgap_cleanup_module();
> +err_unregister:
> + pci_unregister_driver(_driver);
> +err_stop:
> + dgap_stop();
>
> return rc;
> }
> @@ -561,6 +563,21 @@ failed_class:
> return rc;
> }
>
> +static void dgap_stop(void)
> +{
> + unsigned long lock_flags;
> +
> + spin_lock_irqsave(_poll_lock, lock_flags);
> + dgap_poll_stop = 1;
> + spin_unlock_irqrestore(_poll_lock, lock_flags);
> +
> + del_timer_sync(_poll_timer);
> +
> + device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
> + class_destroy(dgap_class);
> + unregister_chrdev(DIGI_DGAP_MAJOR, "dgap");
> +}
> +
> static int dgap_init_one(struct pci_dev *pdev, const struct pci_device_id
> *ent)
> {
> int rc;
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] staging: dgap: introduce dgap_stop()
On Thu, Oct 09, 2014 at 01:40:11PM +0900, Daeseok Youn wrote:
The dgap_init_module() need to unwind for cleanup variables properly.
Because dgap_init_module() calls dgap_cleanup_module() for freeing
variables but this function is possible to free variables
which are not allocated.
Signed-off-by: Daeseok Youn daeseok.y...@gmail.com
---
V2: change ulong which is non-standard to unsigned long.
I think , Dan, in his review of your v1 , asked you to rearrange the functions
and get rid of the forward declarations.
thanks
sudip
drivers/staging/dgap/dgap.c | 27 ++-
1 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 7c79fe6..290ca3b 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -71,6 +71,7 @@ MODULE_DESCRIPTION(Driver for the Digi International EPCA
PCI based product lin
MODULE_SUPPORTED_DEVICE(dgap);
static int dgap_start(void);
+static void dgap_stop(void);
static void dgap_init_globals(void);
static struct board_t *dgap_found_board(struct pci_dev *pdev, int id,
int boardnum);
@@ -479,19 +480,20 @@ static int dgap_init_module(void)
rc = pci_register_driver(dgap_driver);
if (rc)
- goto err_cleanup;
+ goto err_stop;
rc = dgap_create_driver_sysfiles(dgap_driver);
if (rc)
- goto err_cleanup;
+ goto err_unregister;
dgap_driver_state = DRIVER_READY;
return 0;
-err_cleanup:
-
- dgap_cleanup_module();
+err_unregister:
+ pci_unregister_driver(dgap_driver);
+err_stop:
+ dgap_stop();
return rc;
}
@@ -561,6 +563,21 @@ failed_class:
return rc;
}
+static void dgap_stop(void)
+{
+ unsigned long lock_flags;
+
+ spin_lock_irqsave(dgap_poll_lock, lock_flags);
+ dgap_poll_stop = 1;
+ spin_unlock_irqrestore(dgap_poll_lock, lock_flags);
+
+ del_timer_sync(dgap_poll_timer);
+
+ device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
+ class_destroy(dgap_class);
+ unregister_chrdev(DIGI_DGAP_MAJOR, dgap);
+}
+
static int dgap_init_one(struct pci_dev *pdev, const struct pci_device_id
*ent)
{
int rc;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] staging: dgap: introduce dgap_stop()
On Thu, Oct 09, 2014 at 03:19:03PM +0530, Sudip Mukherjee wrote: On Thu, Oct 09, 2014 at 01:40:11PM +0900, Daeseok Youn wrote: The dgap_init_module() need to unwind for cleanup variables properly. Because dgap_init_module() calls dgap_cleanup_module() for freeing variables but this function is possible to free variables which are not allocated. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- V2: change ulong which is non-standard to unsigned long. I think , Dan, in his review of your v1 , asked you to rearrange the functions and get rid of the forward declarations. I was fine with v1 actually... It's a bug fix, and this is staging and there are tons of forward declarations already. Eventually we need to get rid of them but it can be done later. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH V2] staging: dgap: introduce dgap_stop()
Hi, 2014-10-09 19:23 GMT+09:00 Dan Carpenter dan.carpen...@oracle.com: On Thu, Oct 09, 2014 at 03:19:03PM +0530, Sudip Mukherjee wrote: On Thu, Oct 09, 2014 at 01:40:11PM +0900, Daeseok Youn wrote: The dgap_init_module() need to unwind for cleanup variables properly. Because dgap_init_module() calls dgap_cleanup_module() for freeing variables but this function is possible to free variables which are not allocated. Signed-off-by: Daeseok Youn daeseok.y...@gmail.com --- V2: change ulong which is non-standard to unsigned long. I think , Dan, in his review of your v1 , asked you to rearrange the functions and get rid of the forward declarations. I was fine with v1 actually... It's a bug fix, and this is staging and there are tons of forward declarations already. Eventually we need to get rid of them but it can be done later. Yes, I will cleanup as your review. thanks. regards, Daeseok Youn. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] staging: dgap: introduce dgap_stop()
The dgap_init_module() need to unwind for cleanup variables properly.
Because dgap_init_module() calls dgap_cleanup_module() for freeing
variables but this function is possible to free variables
which are not allocated.
Signed-off-by: Daeseok Youn
---
V2: change ulong which is non-standard to "unsigned long".
drivers/staging/dgap/dgap.c | 27 ++-
1 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 7c79fe6..290ca3b 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -71,6 +71,7 @@ MODULE_DESCRIPTION("Driver for the Digi International EPCA
PCI based product lin
MODULE_SUPPORTED_DEVICE("dgap");
static int dgap_start(void);
+static void dgap_stop(void);
static void dgap_init_globals(void);
static struct board_t *dgap_found_board(struct pci_dev *pdev, int id,
int boardnum);
@@ -479,19 +480,20 @@ static int dgap_init_module(void)
rc = pci_register_driver(_driver);
if (rc)
- goto err_cleanup;
+ goto err_stop;
rc = dgap_create_driver_sysfiles(_driver);
if (rc)
- goto err_cleanup;
+ goto err_unregister;
dgap_driver_state = DRIVER_READY;
return 0;
-err_cleanup:
-
- dgap_cleanup_module();
+err_unregister:
+ pci_unregister_driver(_driver);
+err_stop:
+ dgap_stop();
return rc;
}
@@ -561,6 +563,21 @@ failed_class:
return rc;
}
+static void dgap_stop(void)
+{
+ unsigned long lock_flags;
+
+ spin_lock_irqsave(_poll_lock, lock_flags);
+ dgap_poll_stop = 1;
+ spin_unlock_irqrestore(_poll_lock, lock_flags);
+
+ del_timer_sync(_poll_timer);
+
+ device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
+ class_destroy(dgap_class);
+ unregister_chrdev(DIGI_DGAP_MAJOR, "dgap");
+}
+
static int dgap_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
int rc;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2] staging: dgap: introduce dgap_stop()
The dgap_init_module() need to unwind for cleanup variables properly.
Because dgap_init_module() calls dgap_cleanup_module() for freeing
variables but this function is possible to free variables
which are not allocated.
Signed-off-by: Daeseok Youn daeseok.y...@gmail.com
---
V2: change ulong which is non-standard to unsigned long.
drivers/staging/dgap/dgap.c | 27 ++-
1 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 7c79fe6..290ca3b 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -71,6 +71,7 @@ MODULE_DESCRIPTION(Driver for the Digi International EPCA
PCI based product lin
MODULE_SUPPORTED_DEVICE(dgap);
static int dgap_start(void);
+static void dgap_stop(void);
static void dgap_init_globals(void);
static struct board_t *dgap_found_board(struct pci_dev *pdev, int id,
int boardnum);
@@ -479,19 +480,20 @@ static int dgap_init_module(void)
rc = pci_register_driver(dgap_driver);
if (rc)
- goto err_cleanup;
+ goto err_stop;
rc = dgap_create_driver_sysfiles(dgap_driver);
if (rc)
- goto err_cleanup;
+ goto err_unregister;
dgap_driver_state = DRIVER_READY;
return 0;
-err_cleanup:
-
- dgap_cleanup_module();
+err_unregister:
+ pci_unregister_driver(dgap_driver);
+err_stop:
+ dgap_stop();
return rc;
}
@@ -561,6 +563,21 @@ failed_class:
return rc;
}
+static void dgap_stop(void)
+{
+ unsigned long lock_flags;
+
+ spin_lock_irqsave(dgap_poll_lock, lock_flags);
+ dgap_poll_stop = 1;
+ spin_unlock_irqrestore(dgap_poll_lock, lock_flags);
+
+ del_timer_sync(dgap_poll_timer);
+
+ device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
+ class_destroy(dgap_class);
+ unregister_chrdev(DIGI_DGAP_MAJOR, dgap);
+}
+
static int dgap_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
int rc;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
