* [PATCH v2] staging: vc04_services: Avoid NULL comparison
@ 2019-10-07 22:29 Nachammai Karuppiah
2019-10-08 5:45 ` [Outreachy kernel] " Julia Lawall
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Nachammai Karuppiah @ 2019-10-07 22:29 UTC (permalink / raw)
To: Eric Anholt, Stefan Wahren, Greg Kroah-Hartman
Cc: devel, outreachy-kernel, Nachammai Karuppiah
Remove NULL comparison. Issue found using checkpatch.pl
Signed-off-by: Nachammai Karuppiah <nachukannan@gmail.com>
---
Changes in V2
- Remove all NULL comparisons in vc04_services/interface directory.
---
.../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++--
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 28 +++++++++++-----------
.../vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
.../vc04_services/interface/vchiq_arm/vchiq_shim.c | 2 +-
4 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 8dc730c..7cdb21e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -526,11 +526,11 @@ create_pagelist(char __user *buf, size_t count, unsigned short type)
return NULL;
}
- WARN_ON(g_free_fragments == NULL);
+ WARN_ON(!g_free_fragments);
down(&g_free_fragments_mutex);
fragments = g_free_fragments;
- WARN_ON(fragments == NULL);
+ WARN_ON(!fragments);
g_free_fragments = *(char **) g_free_fragments;
up(&g_free_fragments_mutex);
pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index b1595b1..b930148 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -826,8 +826,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
/* Remove all services */
i = 0;
- while ((service = next_service_by_instance(instance->state,
- instance, &i)) != NULL) {
+ while (service = next_service_by_instance(instance->state,
+ instance, &i)) {
status = vchiq_remove_service(service->handle);
unlock_service(service);
if (status != VCHIQ_SUCCESS)
@@ -907,7 +907,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
&args.params, srvstate,
instance, user_service_free);
- if (service != NULL) {
+ if (service) {
user_service->service = service;
user_service->userdata = userdata;
user_service->instance = instance;
@@ -988,7 +988,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
service = find_service_for_instance(instance, handle);
- if (service != NULL) {
+ if (service) {
status = (cmd == VCHIQ_IOC_USE_SERVICE) ?
vchiq_use_service_internal(service) :
vchiq_release_service_internal(service);
@@ -1021,7 +1021,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
service = find_service_for_instance(instance, args.handle);
- if ((service != NULL) && (args.count <= MAX_ELEMENTS)) {
+ if (service && (args.count <= MAX_ELEMENTS)) {
/* Copy elements into kernel space */
struct vchiq_element elements[MAX_ELEMENTS];
@@ -1343,11 +1343,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
spin_unlock(&msg_queue_spinlock);
complete(&user_service->remove_event);
- if (header == NULL)
+ if (!header)
ret = -ENOTCONN;
else if (header->size <= args.bufsize) {
/* Copy to user space if msgbuf is not NULL */
- if ((args.buf == NULL) ||
+ if (!args.buf ||
(copy_to_user((void __user *)args.buf,
header->data,
header->size) == 0)) {
@@ -1426,7 +1426,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
service = find_closed_service_for_instance(instance, handle);
- if (service != NULL) {
+ if (service) {
struct user_service *user_service =
(struct user_service *)service->base.userdata;
close_delivered(user_service);
@@ -2223,13 +2223,13 @@ struct vchiq_state *
vchiq_get_state(void)
{
- if (g_state.remote == NULL)
+ if (!g_state.remote)
printk(KERN_ERR "%s: g_state.remote == NULL\n", __func__);
else if (g_state.remote->initialised != 1)
printk(KERN_NOTICE "%s: g_state.remote->initialised != 1 (%d)\n",
__func__, g_state.remote->initialised);
- return ((g_state.remote != NULL) &&
+ return (g_state.remote &&
(g_state.remote->initialised == 1)) ? &g_state : NULL;
}
@@ -2923,8 +2923,8 @@ vchiq_instance_get_use_count(VCHIQ_INSTANCE_T instance)
int use_count = 0, i;
i = 0;
- while ((service = next_service_by_instance(instance->state,
- instance, &i)) != NULL) {
+ while (service = next_service_by_instance(instance->state,
+ instance, &i)) {
use_count += service->service_use_count;
unlock_service(service);
}
@@ -2950,8 +2950,8 @@ vchiq_instance_set_trace(VCHIQ_INSTANCE_T instance, int trace)
int i;
i = 0;
- while ((service = next_service_by_instance(instance->state,
- instance, &i)) != NULL) {
+ while (service = next_service_by_instance(instance->state,
+ instance, &i)) {
service->trace = trace;
unlock_service(service);
}
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 56a23a2..4c375cd 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -542,7 +542,7 @@ reserve_space(struct vchiq_state *state, size_t space, int is_blocking)
if (space > slot_space) {
struct vchiq_header *header;
/* Fill the remaining space with padding */
- WARN_ON(state->tx_data == NULL);
+ WARN_ON(!state->tx_data);
header = (struct vchiq_header *)
(state->tx_data + (tx_pos & VCHIQ_SLOT_MASK));
header->msgid = VCHIQ_MSGID_PADDING;
@@ -3578,7 +3578,7 @@ void vchiq_log_dump_mem(const char *label, u32 addr, const void *void_mem,
}
*s++ = '\0';
- if ((label != NULL) && (*label != '\0'))
+ if (label && (*label != '\0'))
vchiq_log_trace(VCHIQ_LOG_TRACE,
"%s: %08x: %s", label, addr, line_buf);
else
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
index 17a4f2c..10be23e 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
@@ -628,7 +628,7 @@ int32_t vchi_service_open(VCHI_INSTANCE_T instance_handle,
}
}
- return (service != NULL) ? 0 : -1;
+ return service ? 0 : -1;
}
EXPORT_SYMBOL(vchi_service_open);
--
2.7.4
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] staging: vc04_services: Avoid NULL comparison
2019-10-07 22:29 [PATCH v2] staging: vc04_services: Avoid NULL comparison Nachammai Karuppiah
@ 2019-10-08 5:45 ` Julia Lawall
2019-10-08 8:59 ` kbuild test robot
2019-10-08 12:35 ` Greg Kroah-Hartman
2 siblings, 0 replies; 4+ messages in thread
From: Julia Lawall @ 2019-10-08 5:45 UTC (permalink / raw)
To: Nachammai Karuppiah
Cc: devel, Eric Anholt, outreachy-kernel, Stefan Wahren, Greg Kroah-Hartman
On Mon, 7 Oct 2019, Nachammai Karuppiah wrote:
> Remove NULL comparison. Issue found using checkpatch.pl
This introduces compiler warnings, which you should try very hard not to
do.
julia
>
> Signed-off-by: Nachammai Karuppiah <nachukannan@gmail.com>
>
> ---
>
> Changes in V2
> - Remove all NULL comparisons in vc04_services/interface directory.
> ---
> .../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++--
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 28 +++++++++++-----------
> .../vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
> .../vc04_services/interface/vchiq_arm/vchiq_shim.c | 2 +-
> 4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index 8dc730c..7cdb21e 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -526,11 +526,11 @@ create_pagelist(char __user *buf, size_t count, unsigned short type)
> return NULL;
> }
>
> - WARN_ON(g_free_fragments == NULL);
> + WARN_ON(!g_free_fragments);
>
> down(&g_free_fragments_mutex);
> fragments = g_free_fragments;
> - WARN_ON(fragments == NULL);
> + WARN_ON(!fragments);
> g_free_fragments = *(char **) g_free_fragments;
> up(&g_free_fragments_mutex);
> pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index b1595b1..b930148 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -826,8 +826,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>
> /* Remove all services */
> i = 0;
> - while ((service = next_service_by_instance(instance->state,
> - instance, &i)) != NULL) {
> + while (service = next_service_by_instance(instance->state,
> + instance, &i)) {
> status = vchiq_remove_service(service->handle);
> unlock_service(service);
> if (status != VCHIQ_SUCCESS)
> @@ -907,7 +907,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> &args.params, srvstate,
> instance, user_service_free);
>
> - if (service != NULL) {
> + if (service) {
> user_service->service = service;
> user_service->userdata = userdata;
> user_service->instance = instance;
> @@ -988,7 +988,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
>
> service = find_service_for_instance(instance, handle);
> - if (service != NULL) {
> + if (service) {
> status = (cmd == VCHIQ_IOC_USE_SERVICE) ?
> vchiq_use_service_internal(service) :
> vchiq_release_service_internal(service);
> @@ -1021,7 +1021,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>
> service = find_service_for_instance(instance, args.handle);
>
> - if ((service != NULL) && (args.count <= MAX_ELEMENTS)) {
> + if (service && (args.count <= MAX_ELEMENTS)) {
> /* Copy elements into kernel space */
> struct vchiq_element elements[MAX_ELEMENTS];
>
> @@ -1343,11 +1343,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> spin_unlock(&msg_queue_spinlock);
>
> complete(&user_service->remove_event);
> - if (header == NULL)
> + if (!header)
> ret = -ENOTCONN;
> else if (header->size <= args.bufsize) {
> /* Copy to user space if msgbuf is not NULL */
> - if ((args.buf == NULL) ||
> + if (!args.buf ||
> (copy_to_user((void __user *)args.buf,
> header->data,
> header->size) == 0)) {
> @@ -1426,7 +1426,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
>
> service = find_closed_service_for_instance(instance, handle);
> - if (service != NULL) {
> + if (service) {
> struct user_service *user_service =
> (struct user_service *)service->base.userdata;
> close_delivered(user_service);
> @@ -2223,13 +2223,13 @@ struct vchiq_state *
> vchiq_get_state(void)
> {
>
> - if (g_state.remote == NULL)
> + if (!g_state.remote)
> printk(KERN_ERR "%s: g_state.remote == NULL\n", __func__);
> else if (g_state.remote->initialised != 1)
> printk(KERN_NOTICE "%s: g_state.remote->initialised != 1 (%d)\n",
> __func__, g_state.remote->initialised);
>
> - return ((g_state.remote != NULL) &&
> + return (g_state.remote &&
> (g_state.remote->initialised == 1)) ? &g_state : NULL;
> }
>
> @@ -2923,8 +2923,8 @@ vchiq_instance_get_use_count(VCHIQ_INSTANCE_T instance)
> int use_count = 0, i;
>
> i = 0;
> - while ((service = next_service_by_instance(instance->state,
> - instance, &i)) != NULL) {
> + while (service = next_service_by_instance(instance->state,
> + instance, &i)) {
> use_count += service->service_use_count;
> unlock_service(service);
> }
> @@ -2950,8 +2950,8 @@ vchiq_instance_set_trace(VCHIQ_INSTANCE_T instance, int trace)
> int i;
>
> i = 0;
> - while ((service = next_service_by_instance(instance->state,
> - instance, &i)) != NULL) {
> + while (service = next_service_by_instance(instance->state,
> + instance, &i)) {
> service->trace = trace;
> unlock_service(service);
> }
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> index 56a23a2..4c375cd 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
> @@ -542,7 +542,7 @@ reserve_space(struct vchiq_state *state, size_t space, int is_blocking)
> if (space > slot_space) {
> struct vchiq_header *header;
> /* Fill the remaining space with padding */
> - WARN_ON(state->tx_data == NULL);
> + WARN_ON(!state->tx_data);
> header = (struct vchiq_header *)
> (state->tx_data + (tx_pos & VCHIQ_SLOT_MASK));
> header->msgid = VCHIQ_MSGID_PADDING;
> @@ -3578,7 +3578,7 @@ void vchiq_log_dump_mem(const char *label, u32 addr, const void *void_mem,
> }
> *s++ = '\0';
>
> - if ((label != NULL) && (*label != '\0'))
> + if (label && (*label != '\0'))
> vchiq_log_trace(VCHIQ_LOG_TRACE,
> "%s: %08x: %s", label, addr, line_buf);
> else
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
> index 17a4f2c..10be23e 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
> @@ -628,7 +628,7 @@ int32_t vchi_service_open(VCHI_INSTANCE_T instance_handle,
> }
> }
>
> - return (service != NULL) ? 0 : -1;
> + return service ? 0 : -1;
> }
> EXPORT_SYMBOL(vchi_service_open);
>
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1570487369-35454-1-git-send-email-nachukannan%40gmail.com.
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] staging: vc04_services: Avoid NULL comparison
2019-10-07 22:29 [PATCH v2] staging: vc04_services: Avoid NULL comparison Nachammai Karuppiah
2019-10-08 5:45 ` [Outreachy kernel] " Julia Lawall
@ 2019-10-08 8:59 ` kbuild test robot
2019-10-08 12:35 ` Greg Kroah-Hartman
2 siblings, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2019-10-08 8:59 UTC (permalink / raw)
To: Nachammai Karuppiah
Cc: devel, outreachy-kernel, Greg Kroah-Hartman, Nachammai Karuppiah,
Eric Anholt, kbuild-all, Stefan Wahren
[-- Attachment #1: Type: text/plain, Size: 23251 bytes --]
Hi Nachammai,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
url: https://github.com/0day-ci/linux/commits/Nachammai-Karuppiah/staging-vc04_services-Avoid-NULL-comparison/20191008-143400
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-13) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function 'vchiq_ioctl':
>> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:829:10: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
while (service = next_service_by_instance(instance->state,
^~~~~~~
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function 'vchiq_instance_get_use_count':
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:2926:9: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
while (service = next_service_by_instance(instance->state,
^~~~~~~
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function 'vchiq_instance_set_trace':
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:2953:9: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
while (service = next_service_by_instance(instance->state,
^~~~~~~
vim +829 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
798
799 /****************************************************************************
800 *
801 * vchiq_ioctl
802 *
803 ***************************************************************************/
804 static long
805 vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
806 {
807 VCHIQ_INSTANCE_T instance = file->private_data;
808 VCHIQ_STATUS_T status = VCHIQ_SUCCESS;
809 struct vchiq_service *service = NULL;
810 long ret = 0;
811 int i, rc;
812
813 DEBUG_INITIALISE(g_state.local)
814
815 vchiq_log_trace(vchiq_arm_log_level,
816 "%s - instance %pK, cmd %s, arg %lx",
817 __func__, instance,
818 ((_IOC_TYPE(cmd) == VCHIQ_IOC_MAGIC) &&
819 (_IOC_NR(cmd) <= VCHIQ_IOC_MAX)) ?
820 ioctl_names[_IOC_NR(cmd)] : "<invalid>", arg);
821
822 switch (cmd) {
823 case VCHIQ_IOC_SHUTDOWN:
824 if (!instance->connected)
825 break;
826
827 /* Remove all services */
828 i = 0;
> 829 while (service = next_service_by_instance(instance->state,
830 instance, &i)) {
831 status = vchiq_remove_service(service->handle);
832 unlock_service(service);
833 if (status != VCHIQ_SUCCESS)
834 break;
835 }
836 service = NULL;
837
838 if (status == VCHIQ_SUCCESS) {
839 /* Wake the completion thread and ask it to exit */
840 instance->closing = 1;
841 complete(&instance->insert_event);
842 }
843
844 break;
845
846 case VCHIQ_IOC_CONNECT:
847 if (instance->connected) {
848 ret = -EINVAL;
849 break;
850 }
851 rc = mutex_lock_killable(&instance->state->mutex);
852 if (rc) {
853 vchiq_log_error(vchiq_arm_log_level,
854 "vchiq: connect: could not lock mutex for "
855 "state %d: %d",
856 instance->state->id, rc);
857 ret = -EINTR;
858 break;
859 }
860 status = vchiq_connect_internal(instance->state, instance);
861 mutex_unlock(&instance->state->mutex);
862
863 if (status == VCHIQ_SUCCESS)
864 instance->connected = 1;
865 else
866 vchiq_log_error(vchiq_arm_log_level,
867 "vchiq: could not connect: %d", status);
868 break;
869
870 case VCHIQ_IOC_CREATE_SERVICE: {
871 struct vchiq_create_service args;
872 struct user_service *user_service = NULL;
873 void *userdata;
874 int srvstate;
875
876 if (copy_from_user(&args, (const void __user *)arg,
877 sizeof(args))) {
878 ret = -EFAULT;
879 break;
880 }
881
882 user_service = kmalloc(sizeof(*user_service), GFP_KERNEL);
883 if (!user_service) {
884 ret = -ENOMEM;
885 break;
886 }
887
888 if (args.is_open) {
889 if (!instance->connected) {
890 ret = -ENOTCONN;
891 kfree(user_service);
892 break;
893 }
894 srvstate = VCHIQ_SRVSTATE_OPENING;
895 } else {
896 srvstate =
897 instance->connected ?
898 VCHIQ_SRVSTATE_LISTENING :
899 VCHIQ_SRVSTATE_HIDDEN;
900 }
901
902 userdata = args.params.userdata;
903 args.params.callback = service_callback;
904 args.params.userdata = user_service;
905 service = vchiq_add_service_internal(
906 instance->state,
907 &args.params, srvstate,
908 instance, user_service_free);
909
910 if (service) {
911 user_service->service = service;
912 user_service->userdata = userdata;
913 user_service->instance = instance;
914 user_service->is_vchi = (args.is_vchi != 0);
915 user_service->dequeue_pending = 0;
916 user_service->close_pending = 0;
917 user_service->message_available_pos =
918 instance->completion_remove - 1;
919 user_service->msg_insert = 0;
920 user_service->msg_remove = 0;
921 init_completion(&user_service->insert_event);
922 init_completion(&user_service->remove_event);
923 init_completion(&user_service->close_event);
924
925 if (args.is_open) {
926 status = vchiq_open_service_internal
927 (service, instance->pid);
928 if (status != VCHIQ_SUCCESS) {
929 vchiq_remove_service(service->handle);
930 service = NULL;
931 ret = (status == VCHIQ_RETRY) ?
932 -EINTR : -EIO;
933 break;
934 }
935 }
936
937 if (copy_to_user((void __user *)
938 &(((struct vchiq_create_service __user *)
939 arg)->handle),
940 (const void *)&service->handle,
941 sizeof(service->handle))) {
942 ret = -EFAULT;
943 vchiq_remove_service(service->handle);
944 }
945
946 service = NULL;
947 } else {
948 ret = -EEXIST;
949 kfree(user_service);
950 }
951 } break;
952
953 case VCHIQ_IOC_CLOSE_SERVICE:
954 case VCHIQ_IOC_REMOVE_SERVICE: {
955 VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
956 struct user_service *user_service;
957
958 service = find_service_for_instance(instance, handle);
959 if (!service) {
960 ret = -EINVAL;
961 break;
962 }
963
964 user_service = service->base.userdata;
965
966 /* close_pending is false on first entry, and when the
967 wait in vchiq_close_service has been interrupted. */
968 if (!user_service->close_pending) {
969 status = (cmd == VCHIQ_IOC_CLOSE_SERVICE) ?
970 vchiq_close_service(service->handle) :
971 vchiq_remove_service(service->handle);
972 if (status != VCHIQ_SUCCESS)
973 break;
974 }
975
976 /* close_pending is true once the underlying service
977 has been closed until the client library calls the
978 CLOSE_DELIVERED ioctl, signalling close_event. */
979 if (user_service->close_pending &&
980 wait_for_completion_interruptible(
981 &user_service->close_event))
982 status = VCHIQ_RETRY;
983 break;
984 }
985
986 case VCHIQ_IOC_USE_SERVICE:
987 case VCHIQ_IOC_RELEASE_SERVICE: {
988 VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
989
990 service = find_service_for_instance(instance, handle);
991 if (service) {
992 status = (cmd == VCHIQ_IOC_USE_SERVICE) ?
993 vchiq_use_service_internal(service) :
994 vchiq_release_service_internal(service);
995 if (status != VCHIQ_SUCCESS) {
996 vchiq_log_error(vchiq_susp_log_level,
997 "%s: cmd %s returned error %d for "
998 "service %c%c%c%c:%03d",
999 __func__,
1000 (cmd == VCHIQ_IOC_USE_SERVICE) ?
1001 "VCHIQ_IOC_USE_SERVICE" :
1002 "VCHIQ_IOC_RELEASE_SERVICE",
1003 status,
1004 VCHIQ_FOURCC_AS_4CHARS(
1005 service->base.fourcc),
1006 service->client_id);
1007 ret = -EINVAL;
1008 }
1009 } else
1010 ret = -EINVAL;
1011 } break;
1012
1013 case VCHIQ_IOC_QUEUE_MESSAGE: {
1014 struct vchiq_queue_message args;
1015
1016 if (copy_from_user(&args, (const void __user *)arg,
1017 sizeof(args))) {
1018 ret = -EFAULT;
1019 break;
1020 }
1021
1022 service = find_service_for_instance(instance, args.handle);
1023
1024 if (service && (args.count <= MAX_ELEMENTS)) {
1025 /* Copy elements into kernel space */
1026 struct vchiq_element elements[MAX_ELEMENTS];
1027
1028 if (copy_from_user(elements, args.elements,
1029 args.count * sizeof(struct vchiq_element)) == 0)
1030 status = vchiq_ioc_queue_message
1031 (args.handle,
1032 elements, args.count);
1033 else
1034 ret = -EFAULT;
1035 } else {
1036 ret = -EINVAL;
1037 }
1038 } break;
1039
1040 case VCHIQ_IOC_QUEUE_BULK_TRANSMIT:
1041 case VCHIQ_IOC_QUEUE_BULK_RECEIVE: {
1042 struct vchiq_queue_bulk_transfer args;
1043 struct bulk_waiter_node *waiter = NULL;
1044
1045 VCHIQ_BULK_DIR_T dir =
1046 (cmd == VCHIQ_IOC_QUEUE_BULK_TRANSMIT) ?
1047 VCHIQ_BULK_TRANSMIT : VCHIQ_BULK_RECEIVE;
1048
1049 if (copy_from_user(&args, (const void __user *)arg,
1050 sizeof(args))) {
1051 ret = -EFAULT;
1052 break;
1053 }
1054
1055 service = find_service_for_instance(instance, args.handle);
1056 if (!service) {
1057 ret = -EINVAL;
1058 break;
1059 }
1060
1061 if (args.mode == VCHIQ_BULK_MODE_BLOCKING) {
1062 waiter = kzalloc(sizeof(struct bulk_waiter_node),
1063 GFP_KERNEL);
1064 if (!waiter) {
1065 ret = -ENOMEM;
1066 break;
1067 }
1068
1069 args.userdata = &waiter->bulk_waiter;
1070 } else if (args.mode == VCHIQ_BULK_MODE_WAITING) {
1071 mutex_lock(&instance->bulk_waiter_list_mutex);
1072 list_for_each_entry(waiter, &instance->bulk_waiter_list,
1073 list) {
1074 if (waiter->pid == current->pid) {
1075 list_del(&waiter->list);
1076 break;
1077 }
1078 }
1079 mutex_unlock(&instance->bulk_waiter_list_mutex);
1080 if (!waiter) {
1081 vchiq_log_error(vchiq_arm_log_level,
1082 "no bulk_waiter found for pid %d",
1083 current->pid);
1084 ret = -ESRCH;
1085 break;
1086 }
1087 vchiq_log_info(vchiq_arm_log_level,
1088 "found bulk_waiter %pK for pid %d", waiter,
1089 current->pid);
1090 args.userdata = &waiter->bulk_waiter;
1091 }
1092
1093 status = vchiq_bulk_transfer(args.handle, args.data, args.size,
1094 args.userdata, args.mode, dir);
1095
1096 if (!waiter)
1097 break;
1098
1099 if ((status != VCHIQ_RETRY) || fatal_signal_pending(current) ||
1100 !waiter->bulk_waiter.bulk) {
1101 if (waiter->bulk_waiter.bulk) {
1102 /* Cancel the signal when the transfer
1103 ** completes. */
1104 spin_lock(&bulk_waiter_spinlock);
1105 waiter->bulk_waiter.bulk->userdata = NULL;
1106 spin_unlock(&bulk_waiter_spinlock);
1107 }
1108 kfree(waiter);
1109 } else {
1110 const VCHIQ_BULK_MODE_T mode_waiting =
1111 VCHIQ_BULK_MODE_WAITING;
1112 waiter->pid = current->pid;
1113 mutex_lock(&instance->bulk_waiter_list_mutex);
1114 list_add(&waiter->list, &instance->bulk_waiter_list);
1115 mutex_unlock(&instance->bulk_waiter_list_mutex);
1116 vchiq_log_info(vchiq_arm_log_level,
1117 "saved bulk_waiter %pK for pid %d",
1118 waiter, current->pid);
1119
1120 if (copy_to_user((void __user *)
1121 &(((struct vchiq_queue_bulk_transfer __user *)
1122 arg)->mode),
1123 (const void *)&mode_waiting,
1124 sizeof(mode_waiting)))
1125 ret = -EFAULT;
1126 }
1127 } break;
1128
1129 case VCHIQ_IOC_AWAIT_COMPLETION: {
1130 struct vchiq_await_completion args;
1131
1132 DEBUG_TRACE(AWAIT_COMPLETION_LINE);
1133 if (!instance->connected) {
1134 ret = -ENOTCONN;
1135 break;
1136 }
1137
1138 if (copy_from_user(&args, (const void __user *)arg,
1139 sizeof(args))) {
1140 ret = -EFAULT;
1141 break;
1142 }
1143
1144 mutex_lock(&instance->completion_mutex);
1145
1146 DEBUG_TRACE(AWAIT_COMPLETION_LINE);
1147 while ((instance->completion_remove ==
1148 instance->completion_insert)
1149 && !instance->closing) {
1150 int rc;
1151
1152 DEBUG_TRACE(AWAIT_COMPLETION_LINE);
1153 mutex_unlock(&instance->completion_mutex);
1154 rc = wait_for_completion_interruptible(
1155 &instance->insert_event);
1156 mutex_lock(&instance->completion_mutex);
1157 if (rc) {
1158 DEBUG_TRACE(AWAIT_COMPLETION_LINE);
1159 vchiq_log_info(vchiq_arm_log_level,
1160 "AWAIT_COMPLETION interrupted");
1161 ret = -EINTR;
1162 break;
1163 }
1164 }
1165 DEBUG_TRACE(AWAIT_COMPLETION_LINE);
1166
1167 if (ret == 0) {
1168 int msgbufcount = args.msgbufcount;
1169 int remove = instance->completion_remove;
1170
1171 for (ret = 0; ret < args.count; ret++) {
1172 struct vchiq_completion_data *completion;
1173 struct vchiq_service *service;
1174 struct user_service *user_service;
1175 struct vchiq_header *header;
1176
1177 if (remove == instance->completion_insert)
1178 break;
1179
1180 completion = &instance->completions[
1181 remove & (MAX_COMPLETIONS - 1)];
1182
1183 /*
1184 * A read memory barrier is needed to stop
1185 * prefetch of a stale completion record
1186 */
1187 rmb();
1188
1189 service = completion->service_userdata;
1190 user_service = service->base.userdata;
1191 completion->service_userdata =
1192 user_service->userdata;
1193
1194 header = completion->header;
1195 if (header) {
1196 void __user *msgbuf;
1197 int msglen;
1198
1199 msglen = header->size +
1200 sizeof(struct vchiq_header);
1201 /* This must be a VCHIQ-style service */
1202 if (args.msgbufsize < msglen) {
1203 vchiq_log_error(
1204 vchiq_arm_log_level,
1205 "header %pK: msgbufsize %x < msglen %x",
1206 header, args.msgbufsize,
1207 msglen);
1208 WARN(1, "invalid message "
1209 "size\n");
1210 if (ret == 0)
1211 ret = -EMSGSIZE;
1212 break;
1213 }
1214 if (msgbufcount <= 0)
1215 /* Stall here for lack of a
1216 ** buffer for the message. */
1217 break;
1218 /* Get the pointer from user space */
1219 msgbufcount--;
1220 if (copy_from_user(&msgbuf,
1221 (const void __user *)
1222 &args.msgbufs[msgbufcount],
1223 sizeof(msgbuf))) {
1224 if (ret == 0)
1225 ret = -EFAULT;
1226 break;
1227 }
1228
1229 /* Copy the message to user space */
1230 if (copy_to_user(msgbuf, header,
1231 msglen)) {
1232 if (ret == 0)
1233 ret = -EFAULT;
1234 break;
1235 }
1236
1237 /* Now it has been copied, the message
1238 ** can be released. */
1239 vchiq_release_message(service->handle,
1240 header);
1241
1242 /* The completion must point to the
1243 ** msgbuf. */
1244 completion->header = msgbuf;
1245 }
1246
1247 if ((completion->reason ==
1248 VCHIQ_SERVICE_CLOSED) &&
1249 !instance->use_close_delivered)
1250 unlock_service(service);
1251
1252 if (copy_to_user((void __user *)(
1253 (size_t)args.buf + ret *
1254 sizeof(struct vchiq_completion_data)),
1255 completion,
1256 sizeof(struct vchiq_completion_data))) {
1257 if (ret == 0)
1258 ret = -EFAULT;
1259 break;
1260 }
1261
1262 /*
1263 * Ensure that the above copy has completed
1264 * before advancing the remove pointer.
1265 */
1266 mb();
1267 remove++;
1268 instance->completion_remove = remove;
1269 }
1270
1271 if (msgbufcount != args.msgbufcount) {
1272 if (copy_to_user((void __user *)
1273 &((struct vchiq_await_completion *)arg)
1274 ->msgbufcount,
1275 &msgbufcount,
1276 sizeof(msgbufcount))) {
1277 ret = -EFAULT;
1278 }
1279 }
1280 }
1281
1282 if (ret)
1283 complete(&instance->remove_event);
1284 mutex_unlock(&instance->completion_mutex);
1285 DEBUG_TRACE(AWAIT_COMPLETION_LINE);
1286 } break;
1287
1288 case VCHIQ_IOC_DEQUEUE_MESSAGE: {
1289 struct vchiq_dequeue_message args;
1290 struct user_service *user_service;
1291 struct vchiq_header *header;
1292
1293 DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
1294 if (copy_from_user(&args, (const void __user *)arg,
1295 sizeof(args))) {
1296 ret = -EFAULT;
1297 break;
1298 }
1299 service = find_service_for_instance(instance, args.handle);
1300 if (!service) {
1301 ret = -EINVAL;
1302 break;
1303 }
1304 user_service = (struct user_service *)service->base.userdata;
1305 if (user_service->is_vchi == 0) {
1306 ret = -EINVAL;
1307 break;
1308 }
1309
1310 spin_lock(&msg_queue_spinlock);
1311 if (user_service->msg_remove == user_service->msg_insert) {
1312 if (!args.blocking) {
1313 spin_unlock(&msg_queue_spinlock);
1314 DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
1315 ret = -EWOULDBLOCK;
1316 break;
1317 }
1318 user_service->dequeue_pending = 1;
1319 do {
1320 spin_unlock(&msg_queue_spinlock);
1321 DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
1322 if (wait_for_completion_interruptible(
1323 &user_service->insert_event)) {
1324 vchiq_log_info(vchiq_arm_log_level,
1325 "DEQUEUE_MESSAGE interrupted");
1326 ret = -EINTR;
1327 break;
1328 }
1329 spin_lock(&msg_queue_spinlock);
1330 } while (user_service->msg_remove ==
1331 user_service->msg_insert);
1332
1333 if (ret)
1334 break;
1335 }
1336
1337 BUG_ON((int)(user_service->msg_insert -
1338 user_service->msg_remove) < 0);
1339
1340 header = user_service->msg_queue[user_service->msg_remove &
1341 (MSG_QUEUE_SIZE - 1)];
1342 user_service->msg_remove++;
1343 spin_unlock(&msg_queue_spinlock);
1344
1345 complete(&user_service->remove_event);
1346 if (!header)
1347 ret = -ENOTCONN;
1348 else if (header->size <= args.bufsize) {
1349 /* Copy to user space if msgbuf is not NULL */
1350 if (!args.buf ||
1351 (copy_to_user((void __user *)args.buf,
1352 header->data,
1353 header->size) == 0)) {
1354 ret = header->size;
1355 vchiq_release_message(
1356 service->handle,
1357 header);
1358 } else
1359 ret = -EFAULT;
1360 } else {
1361 vchiq_log_error(vchiq_arm_log_level,
1362 "header %pK: bufsize %x < size %x",
1363 header, args.bufsize, header->size);
1364 WARN(1, "invalid size\n");
1365 ret = -EMSGSIZE;
1366 }
1367 DEBUG_TRACE(DEQUEUE_MESSAGE_LINE);
1368 } break;
1369
1370 case VCHIQ_IOC_GET_CLIENT_ID: {
1371 VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
1372
1373 ret = vchiq_get_client_id(handle);
1374 } break;
1375
1376 case VCHIQ_IOC_GET_CONFIG: {
1377 struct vchiq_get_config args;
1378 struct vchiq_config config;
1379
1380 if (copy_from_user(&args, (const void __user *)arg,
1381 sizeof(args))) {
1382 ret = -EFAULT;
1383 break;
1384 }
1385 if (args.config_size > sizeof(config)) {
1386 ret = -EINVAL;
1387 break;
1388 }
1389
1390 vchiq_get_config(&config);
1391 if (copy_to_user(args.pconfig, &config, args.config_size)) {
1392 ret = -EFAULT;
1393 break;
1394 }
1395 } break;
1396
1397 case VCHIQ_IOC_SET_SERVICE_OPTION: {
1398 struct vchiq_set_service_option args;
1399
1400 if (copy_from_user(&args, (const void __user *)arg,
1401 sizeof(args))) {
1402 ret = -EFAULT;
1403 break;
1404 }
1405
1406 service = find_service_for_instance(instance, args.handle);
1407 if (!service) {
1408 ret = -EINVAL;
1409 break;
1410 }
1411
1412 status = vchiq_set_service_option(
1413 args.handle, args.option, args.value);
1414 } break;
1415
1416 case VCHIQ_IOC_LIB_VERSION: {
1417 unsigned int lib_version = (unsigned int)arg;
1418
1419 if (lib_version < VCHIQ_VERSION_MIN)
1420 ret = -EINVAL;
1421 else if (lib_version >= VCHIQ_VERSION_CLOSE_DELIVERED)
1422 instance->use_close_delivered = 1;
1423 } break;
1424
1425 case VCHIQ_IOC_CLOSE_DELIVERED: {
1426 VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
1427
1428 service = find_closed_service_for_instance(instance, handle);
1429 if (service) {
1430 struct user_service *user_service =
1431 (struct user_service *)service->base.userdata;
1432 close_delivered(user_service);
1433 } else
1434 ret = -EINVAL;
1435 } break;
1436
1437 default:
1438 ret = -ENOTTY;
1439 break;
1440 }
1441
1442 if (service)
1443 unlock_service(service);
1444
1445 if (ret == 0) {
1446 if (status == VCHIQ_ERROR)
1447 ret = -EIO;
1448 else if (status == VCHIQ_RETRY)
1449 ret = -EINTR;
1450 }
1451
1452 if ((status == VCHIQ_SUCCESS) && (ret < 0) && (ret != -EINTR) &&
1453 (ret != -EWOULDBLOCK))
1454 vchiq_log_info(vchiq_arm_log_level,
1455 " ioctl instance %pK, cmd %s -> status %d, %ld",
1456 instance,
1457 (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
1458 ioctl_names[_IOC_NR(cmd)] :
1459 "<invalid>",
1460 status, ret);
1461 else
1462 vchiq_log_trace(vchiq_arm_log_level,
1463 " ioctl instance %pK, cmd %s -> status %d, %ld",
1464 instance,
1465 (_IOC_NR(cmd) <= VCHIQ_IOC_MAX) ?
1466 ioctl_names[_IOC_NR(cmd)] :
1467 "<invalid>",
1468 status, ret);
1469
1470 return ret;
1471 }
1472
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70194 bytes --]
[-- Attachment #3: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] staging: vc04_services: Avoid NULL comparison
2019-10-07 22:29 [PATCH v2] staging: vc04_services: Avoid NULL comparison Nachammai Karuppiah
2019-10-08 5:45 ` [Outreachy kernel] " Julia Lawall
2019-10-08 8:59 ` kbuild test robot
@ 2019-10-08 12:35 ` Greg Kroah-Hartman
2 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-08 12:35 UTC (permalink / raw)
To: Nachammai Karuppiah; +Cc: devel, Eric Anholt, Stefan Wahren, outreachy-kernel
On Mon, Oct 07, 2019 at 03:29:28PM -0700, Nachammai Karuppiah wrote:
> Remove NULL comparison. Issue found using checkpatch.pl
>
> Signed-off-by: Nachammai Karuppiah <nachukannan@gmail.com>
>
> ---
>
> Changes in V2
> - Remove all NULL comparisons in vc04_services/interface directory.
> ---
> .../interface/vchiq_arm/vchiq_2835_arm.c | 4 ++--
> .../vc04_services/interface/vchiq_arm/vchiq_arm.c | 28 +++++++++++-----------
> .../vc04_services/interface/vchiq_arm/vchiq_core.c | 4 ++--
> .../vc04_services/interface/vchiq_arm/vchiq_shim.c | 2 +-
> 4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index 8dc730c..7cdb21e 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -526,11 +526,11 @@ create_pagelist(char __user *buf, size_t count, unsigned short type)
> return NULL;
> }
>
> - WARN_ON(g_free_fragments == NULL);
> + WARN_ON(!g_free_fragments);
>
> down(&g_free_fragments_mutex);
> fragments = g_free_fragments;
> - WARN_ON(fragments == NULL);
> + WARN_ON(!fragments);
> g_free_fragments = *(char **) g_free_fragments;
> up(&g_free_fragments_mutex);
> pagelist->type = PAGELIST_READ_WITH_FRAGMENTS +
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index b1595b1..b930148 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -826,8 +826,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>
> /* Remove all services */
> i = 0;
> - while ((service = next_service_by_instance(instance->state,
> - instance, &i)) != NULL) {
> + while (service = next_service_by_instance(instance->state,
> + instance, &i)) {
As you can see, this added a build warning. Please always be careful
about your patches to not have them do that :(
Please fix up and resend.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-08 12:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07 22:29 [PATCH v2] staging: vc04_services: Avoid NULL comparison Nachammai Karuppiah
2019-10-08 5:45 ` [Outreachy kernel] " Julia Lawall
2019-10-08 8:59 ` kbuild test robot
2019-10-08 12:35 ` Greg Kroah-Hartman
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).