* [PATCH 0/1] staging: vc04_services: Use %p4cc to print fourcc
@ 2023-10-25 6:07 Umang Jain
2023-10-25 6:07 ` [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes Umang Jain
2023-10-25 11:29 ` [PATCH 0/1] staging: vc04_services: Use %p4cc to print fourcc Stefan Wahren
0 siblings, 2 replies; 8+ messages in thread
From: Umang Jain @ 2023-10-25 6:07 UTC (permalink / raw)
To: linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel
Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham,
Laurent Pinchart, Ricardo B . Marliere, Sakari Ailus, Umang Jain
The following patch drop VCHIQ_FOURCC_AS_4CHARS macro in favour of %p4cc
format modifier to print FourCC codes in the logs.
*Before this patch*
`mailbox: vchiq_core_msg debug: Sent Msg DATA(5) to AUDS s:1 d:62 len:20`
*After this patch*
bcm2835_vchiq 3f00b840.mailbox: vchiq_core_msg debug: Sent Msg DATA(5) to SDUA little-endian (0x41554453) s:1 d:62 len:20
The inversion of AUDS to SDUA as per usage of %p4cc
Does it hamper readability ? Feedback is appreciated.
As documented in the commit message, the 'entity' char array length is
increased to hold more characters for the log output. Not doing so,
causes kernel stack corruption at runtime.
Based on top of:
- [PATCH v2 0/8] staging: vc04: Drop custom logging based on printk
Umang Jain (1):
staging: vc04_services: Use %p4cc format modifier to print FourCC
codes
.../interface/vchiq_arm/vchiq_arm.c | 20 +++++-----
.../interface/vchiq_arm/vchiq_core.c | 40 +++++++++----------
.../interface/vchiq_arm/vchiq_core.h | 6 ---
.../interface/vchiq_arm/vchiq_dev.c | 7 ++--
4 files changed, 33 insertions(+), 40 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes 2023-10-25 6:07 [PATCH 0/1] staging: vc04_services: Use %p4cc to print fourcc Umang Jain @ 2023-10-25 6:07 ` Umang Jain 2023-10-25 7:10 ` Dan Carpenter 2023-10-25 11:29 ` [PATCH 0/1] staging: vc04_services: Use %p4cc to print fourcc Stefan Wahren 1 sibling, 1 reply; 8+ messages in thread From: Umang Jain @ 2023-10-25 6:07 UTC (permalink / raw) To: linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham, Laurent Pinchart, Ricardo B . Marliere, Sakari Ailus, Umang Jain Drop VCHIQ_FOURCC_AS_4CHARS macro in favour of %p4cc format modifier to print FourCC codes in the logs. vchiq_use_internal() and vchiq_release_internal() uses entity character-array to store a transient string that contains a FourCC code. Increase the length of entity array(to 64 bytes) since %p4cc requires more bytes to hold the output characters. Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- .../interface/vchiq_arm/vchiq_arm.c | 20 +++++----- .../interface/vchiq_arm/vchiq_core.c | 40 +++++++++---------- .../interface/vchiq_arm/vchiq_core.h | 6 --- .../interface/vchiq_arm/vchiq_dev.c | 7 ++-- 4 files changed, 33 insertions(+), 40 deletions(-) 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 fc6d33ec5e95..de6a24304a4d 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -1441,7 +1441,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, { struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); int ret = 0; - char entity[16]; + char entity[64]; int *entity_uc; int local_uc; @@ -1454,8 +1454,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, sprintf(entity, "VCHIQ: "); entity_uc = &arm_state->peer_use_count; } else if (service) { - sprintf(entity, "%c%c%c%c:%03d", - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), + sprintf(entity, "%p4cc:%03d", + &service->base.fourcc, service->client_id); entity_uc = &service->service_use_count; } else { @@ -1497,7 +1497,7 @@ vchiq_release_internal(struct vchiq_state *state, struct vchiq_service *service) { struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); int ret = 0; - char entity[16]; + char entity[64]; int *entity_uc; if (!arm_state) { @@ -1506,8 +1506,8 @@ vchiq_release_internal(struct vchiq_state *state, struct vchiq_service *service) } if (service) { - sprintf(entity, "%c%c%c%c:%03d", - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), + sprintf(entity, "%p4cc:%03d", + &service->base.fourcc, service->client_id); entity_uc = &service->service_use_count; } else { @@ -1713,8 +1713,8 @@ vchiq_dump_service_use_state(struct vchiq_state *state) for (i = 0; i < found; i++) { vchiq_log_warning(state->dev, VCHIQ_SUSPEND, - "%c%c%c%c:%d service count %d %s", - VCHIQ_FOURCC_AS_4CHARS(service_data[i].fourcc), + "%p4cc:%d service count %d %s", + &service_data[i].fourcc, service_data[i].clientid, service_data[i].use_count, service_data[i].use_count ? nz : ""); } @@ -1743,8 +1743,8 @@ vchiq_check_service(struct vchiq_service *service) if (ret) { vchiq_log_error(service->state->dev, VCHIQ_SUSPEND, - "%s ERROR - %c%c%c%c:%d service count %d, state count %d", __func__, - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), service->client_id, + "%s ERROR - %p4cc:%d service count %d, state count %d", __func__, + &service->base.fourcc, service->client_id, service->service_use_count, arm_state->videocore_use_count); vchiq_dump_service_use_state(service->state); } 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 45ba8f509a84..39b857da2d42 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -1112,9 +1112,9 @@ queue_message(struct vchiq_state *state, struct vchiq_service *service, : VCHIQ_MAKE_FOURCC('?', '?', '?', '?'); vchiq_log_debug(state->dev, VCHIQ_CORE_MSG, - "Sent Msg %s(%u) to %c%c%c%c s:%u d:%d len:%zu", + "Sent Msg %s(%u) to %p4cc s:%u d:%d len:%zu", msg_type_str(VCHIQ_MSG_TYPE(msgid)), VCHIQ_MSG_TYPE(msgid), - VCHIQ_FOURCC_AS_4CHARS(svc_fourcc), VCHIQ_MSG_SRCPORT(msgid), + &svc_fourcc, VCHIQ_MSG_SRCPORT(msgid), VCHIQ_MSG_DSTPORT(msgid), size); } @@ -1206,9 +1206,9 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service, : VCHIQ_MAKE_FOURCC('?', '?', '?', '?'); vchiq_log_trace(state->dev, VCHIQ_SYNC, - "Sent Sync Msg %s(%u) to %c%c%c%c s:%u d:%d len:%d", + "Sent Sync Msg %s(%u) to %p4cc s:%u d:%d len:%d", msg_type_str(VCHIQ_MSG_TYPE(msgid)), VCHIQ_MSG_TYPE(msgid), - VCHIQ_FOURCC_AS_4CHARS(svc_fourcc), VCHIQ_MSG_SRCPORT(msgid), + &svc_fourcc, VCHIQ_MSG_SRCPORT(msgid), VCHIQ_MSG_DSTPORT(msgid), size); remote_event_signal(&state->remote->sync_trigger); @@ -1449,9 +1449,9 @@ abort_outstanding_bulks(struct vchiq_service *service, vchiq_complete_bulk(service->instance, bulk); vchiq_log_debug(service->state->dev, VCHIQ_CORE_MSG, - "%s %c%c%c%c d:%d ABORTED - tx len:%d, rx len:%d", + "%s %p4cc d:%d ABORTED - tx len:%d, rx len:%d", is_tx ? "Send Bulk to" : "Recv Bulk from", - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), + &service->base.fourcc, service->remoteport, bulk->size, bulk->remote_size); } else { /* fabricate a matching dummy bulk */ @@ -1485,8 +1485,8 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header) payload = (struct vchiq_open_payload *)header->data; fourcc = payload->fourcc; - vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: prs OPEN@%pK (%d->'%c%c%c%c')", - state->id, header, localport, VCHIQ_FOURCC_AS_4CHARS(fourcc)); + vchiq_log_debug(state->dev, VCHIQ_CORE, "%d: prs OPEN@%pK (%d->'%p4cc')", + state->id, header, localport, &fourcc); service = get_listening_service(state, fourcc); if (!service) @@ -1498,8 +1498,8 @@ parse_open(struct vchiq_state *state, struct vchiq_header *header) if ((service->version < version_min) || (version < service->version_min)) { /* Version mismatch */ - dev_err(state->dev, "%d: service %d (%c%c%c%c) version mismatch - local (%d, min %d) vs. remote (%d, min %d)", - state->id, service->localport, VCHIQ_FOURCC_AS_4CHARS(fourcc), + dev_err(state->dev, "%d: service %d (%p4cc) version mismatch - local (%d, min %d) vs. remote (%d, min %d)", + state->id, service->localport, &fourcc, service->version, service->version_min, version, version_min); vchiq_service_put(service); service = NULL; @@ -1632,8 +1632,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header) : VCHIQ_MAKE_FOURCC('?', '?', '?', '?'); vchiq_log_debug(state->dev, VCHIQ_CORE_MSG, - "Rcvd Msg %s(%u) from %c%c%c%c s:%d d:%d len:%d", - msg_type_str(type), type, VCHIQ_FOURCC_AS_4CHARS(svc_fourcc), + "Rcvd Msg %s(%u) from %p4cc s:%d d:%d len:%d", + msg_type_str(type), type, &svc_fourcc, remoteport, localport, size); if (size > 0) vchiq_log_dump_mem(state->dev, "Rcvd", 0, header->data, min(16, size)); @@ -1683,8 +1683,8 @@ parse_message(struct vchiq_state *state, struct vchiq_header *header) if (vchiq_close_service_internal(service, CLOSE_RECVD) == -EAGAIN) goto bail_not_ready; - vchiq_log_debug(state->dev, VCHIQ_CORE, "Close Service %c%c%c%c s:%u d:%d", - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), + vchiq_log_debug(state->dev, VCHIQ_CORE, "Close Service %p4cc s:%u d:%d", + &service->base.fourcc, service->localport, service->remoteport); break; case VCHIQ_MSG_DATA: @@ -2044,8 +2044,8 @@ sync_func(void *v) : VCHIQ_MAKE_FOURCC('?', '?', '?', '?'); vchiq_log_trace(state->dev, VCHIQ_SYNC, - "Rcvd Msg %s from %c%c%c%c s:%d d:%d len:%d", - msg_type_str(type), VCHIQ_FOURCC_AS_4CHARS(svc_fourcc), + "Rcvd Msg %s from %p4cc s:%d d:%d len:%d", + msg_type_str(type), &svc_fourcc, remoteport, localport, size); if (size > 0) vchiq_log_dump_mem(state->dev, "Rcvd", 0, header->data, min(16, size)); @@ -2462,9 +2462,9 @@ vchiq_add_service_internal(struct vchiq_state *state, /* Bring this service online */ set_service_state(service, srvstate); - vchiq_log_debug(state->dev, VCHIQ_CORE_MSG, "%s Service %c%c%c%c SrcPort:%d", + vchiq_log_debug(state->dev, VCHIQ_CORE_MSG, "%s Service %p4cc SrcPort:%d", (srvstate == VCHIQ_SRVSTATE_OPENING) ? "Open" : "Add", - VCHIQ_FOURCC_AS_4CHARS(params->fourcc), service->localport); + ¶ms->fourcc, service->localport); /* Don't unlock the service - leave it with a ref_count of 1. */ @@ -3543,8 +3543,8 @@ int vchiq_dump_service_state(void *dump_context, struct vchiq_service *service) } len += scnprintf(buf + len, sizeof(buf) - len, - " '%c%c%c%c' remote %s (msg use %d/%d, slot use %d/%d)", - VCHIQ_FOURCC_AS_4CHARS(fourcc), remoteport, + " '%p4cc' remote %s (msg use %d/%d, slot use %d/%d)", + &fourcc, remoteport, quota->message_use_count, quota->message_quota, quota->slot_use_count, quota->slot_quota); diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h index 10451d6765bd..161358db457c 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -74,12 +74,6 @@ static inline const char *log_category_str(enum vchiq_log_category c) #define VCHIQ_SLOT_ZERO_SLOTS DIV_ROUND_UP(sizeof(struct vchiq_slot_zero), \ VCHIQ_SLOT_SIZE) -#define VCHIQ_FOURCC_AS_4CHARS(fourcc) \ - ((fourcc) >> 24) & 0xff, \ - ((fourcc) >> 16) & 0xff, \ - ((fourcc) >> 8) & 0xff, \ - (fourcc) & 0xff - #define BITSET_SIZE(b) ((b + 31) >> 5) #define BITSET_WORD(b) (b >> 5) #define BITSET_BIT(b) (1 << (b & 31)) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c index b6b5804689fa..0bc93f48c14c 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c @@ -701,13 +701,12 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) vchiq_release_service_internal(service); if (ret) { vchiq_log_error(instance->state->dev, VCHIQ_SUSPEND, - "%s: cmd %s returned error %ld for service %c%c%c%c:%03d", + "%s: cmd %s returned error %ld for service %p4cc:%03d", __func__, (cmd == VCHIQ_IOC_USE_SERVICE) ? "VCHIQ_IOC_USE_SERVICE" : "VCHIQ_IOC_RELEASE_SERVICE", - ret, - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), - service->client_id); + ret, &service->base.fourcc, + service->client_id); } } else { ret = -EINVAL; -- 2.41.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes 2023-10-25 6:07 ` [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes Umang Jain @ 2023-10-25 7:10 ` Dan Carpenter 2023-10-25 9:28 ` Sakari Ailus 0 siblings, 1 reply; 8+ messages in thread From: Dan Carpenter @ 2023-10-25 7:10 UTC (permalink / raw) To: Umang Jain Cc: linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel, Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham, Laurent Pinchart, Ricardo B . Marliere, Sakari Ailus On Wed, Oct 25, 2023 at 02:07:17AM -0400, Umang Jain wrote: > Drop VCHIQ_FOURCC_AS_4CHARS macro in favour of %p4cc format > modifier to print FourCC codes in the logs. > > vchiq_use_internal() and vchiq_release_internal() uses entity > character-array to store a transient string that contains > a FourCC code. Increase the length of entity array(to 64 bytes) > since %p4cc requires more bytes to hold the output characters. > > Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > .../interface/vchiq_arm/vchiq_arm.c | 20 +++++----- > .../interface/vchiq_arm/vchiq_core.c | 40 +++++++++---------- > .../interface/vchiq_arm/vchiq_core.h | 6 --- > .../interface/vchiq_arm/vchiq_dev.c | 7 ++-- > 4 files changed, 33 insertions(+), 40 deletions(-) > > 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 fc6d33ec5e95..de6a24304a4d 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > @@ -1441,7 +1441,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, > { > struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); > int ret = 0; > - char entity[16]; > + char entity[64]; > int *entity_uc; > int local_uc; > > @@ -1454,8 +1454,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, > sprintf(entity, "VCHIQ: "); > entity_uc = &arm_state->peer_use_count; > } else if (service) { > - sprintf(entity, "%c%c%c%c:%03d", > - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), > + sprintf(entity, "%p4cc:%03d", Not related to your patch but these sprintfs() make me very uncomfortable. KTODO: change sprintf() to snprintf() in staging/vc04_services/ regards, dan carpenter ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes 2023-10-25 7:10 ` Dan Carpenter @ 2023-10-25 9:28 ` Sakari Ailus 2023-10-25 9:34 ` Umang Jain 0 siblings, 1 reply; 8+ messages in thread From: Sakari Ailus @ 2023-10-25 9:28 UTC (permalink / raw) To: Dan Carpenter, Umang Jain, linux-staging Cc: linux-rpi-kernel, linux-media, linux-arm-kernel, Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham, Laurent Pinchart, Ricardo B . Marliere On Wed, Oct 25, 2023 at 10:10:08AM +0300, Dan Carpenter wrote: > On Wed, Oct 25, 2023 at 02:07:17AM -0400, Umang Jain wrote: > > Drop VCHIQ_FOURCC_AS_4CHARS macro in favour of %p4cc format > > modifier to print FourCC codes in the logs. > > > > vchiq_use_internal() and vchiq_release_internal() uses entity > > character-array to store a transient string that contains > > a FourCC code. Increase the length of entity array(to 64 bytes) > > since %p4cc requires more bytes to hold the output characters. > > > > Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > --- > > .../interface/vchiq_arm/vchiq_arm.c | 20 +++++----- > > .../interface/vchiq_arm/vchiq_core.c | 40 +++++++++---------- > > .../interface/vchiq_arm/vchiq_core.h | 6 --- > > .../interface/vchiq_arm/vchiq_dev.c | 7 ++-- > > 4 files changed, 33 insertions(+), 40 deletions(-) > > > > 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 fc6d33ec5e95..de6a24304a4d 100644 > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > @@ -1441,7 +1441,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, > > { > > struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); > > int ret = 0; > > - char entity[16]; > > + char entity[64]; > > int *entity_uc; > > int local_uc; > > > > @@ -1454,8 +1454,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, > > sprintf(entity, "VCHIQ: "); > > entity_uc = &arm_state->peer_use_count; > > } else if (service) { > > - sprintf(entity, "%c%c%c%c:%03d", > > - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), > > + sprintf(entity, "%p4cc:%03d", > > Not related to your patch but these sprintfs() make me very > uncomfortable. > > KTODO: change sprintf() to snprintf() in staging/vc04_services/ Umang: how about one patch on top of this? :-) There are just five instances of it. -- Sakari Ailus ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes 2023-10-25 9:28 ` Sakari Ailus @ 2023-10-25 9:34 ` Umang Jain 2023-10-25 9:46 ` Greg Kroah-Hartman 2023-10-25 11:12 ` Ricardo B. Marliere 0 siblings, 2 replies; 8+ messages in thread From: Umang Jain @ 2023-10-25 9:34 UTC (permalink / raw) To: Sakari Ailus, Dan Carpenter, linux-staging, Ricardo B . Marliere Cc: linux-rpi-kernel, linux-media, linux-arm-kernel, Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham, Laurent Pinchart On 10/25/23 2:58 PM, Sakari Ailus wrote: > On Wed, Oct 25, 2023 at 10:10:08AM +0300, Dan Carpenter wrote: >> On Wed, Oct 25, 2023 at 02:07:17AM -0400, Umang Jain wrote: >>> Drop VCHIQ_FOURCC_AS_4CHARS macro in favour of %p4cc format >>> modifier to print FourCC codes in the logs. >>> >>> vchiq_use_internal() and vchiq_release_internal() uses entity >>> character-array to store a transient string that contains >>> a FourCC code. Increase the length of entity array(to 64 bytes) >>> since %p4cc requires more bytes to hold the output characters. >>> >>> Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >>> --- >>> .../interface/vchiq_arm/vchiq_arm.c | 20 +++++----- >>> .../interface/vchiq_arm/vchiq_core.c | 40 +++++++++---------- >>> .../interface/vchiq_arm/vchiq_core.h | 6 --- >>> .../interface/vchiq_arm/vchiq_dev.c | 7 ++-- >>> 4 files changed, 33 insertions(+), 40 deletions(-) >>> >>> 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 fc6d33ec5e95..de6a24304a4d 100644 >>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c >>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c >>> @@ -1441,7 +1441,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, >>> { >>> struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); >>> int ret = 0; >>> - char entity[16]; >>> + char entity[64]; >>> int *entity_uc; >>> int local_uc; >>> >>> @@ -1454,8 +1454,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, >>> sprintf(entity, "VCHIQ: "); >>> entity_uc = &arm_state->peer_use_count; >>> } else if (service) { >>> - sprintf(entity, "%c%c%c%c:%03d", >>> - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), >>> + sprintf(entity, "%p4cc:%03d", >> Not related to your patch but these sprintfs() make me very >> uncomfortable. >> >> KTODO: change sprintf() to snprintf() in staging/vc04_services/ > Umang: how about one patch on top of this? :-) There are just five > instances of it. Ricardo, how about this? Do you want to take a swing at this ? :-) And send a v2. ? > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes 2023-10-25 9:34 ` Umang Jain @ 2023-10-25 9:46 ` Greg Kroah-Hartman 2023-10-25 11:12 ` Ricardo B. Marliere 1 sibling, 0 replies; 8+ messages in thread From: Greg Kroah-Hartman @ 2023-10-25 9:46 UTC (permalink / raw) To: Umang Jain Cc: Sakari Ailus, Dan Carpenter, linux-staging, Ricardo B . Marliere, linux-rpi-kernel, linux-media, linux-arm-kernel, Stefan Wahren, Dan Carpenter, Kieran Bingham, Laurent Pinchart On Wed, Oct 25, 2023 at 03:04:04PM +0530, Umang Jain wrote: > > > On 10/25/23 2:58 PM, Sakari Ailus wrote: > > On Wed, Oct 25, 2023 at 10:10:08AM +0300, Dan Carpenter wrote: > > > On Wed, Oct 25, 2023 at 02:07:17AM -0400, Umang Jain wrote: > > > > Drop VCHIQ_FOURCC_AS_4CHARS macro in favour of %p4cc format > > > > modifier to print FourCC codes in the logs. > > > > > > > > vchiq_use_internal() and vchiq_release_internal() uses entity > > > > character-array to store a transient string that contains > > > > a FourCC code. Increase the length of entity array(to 64 bytes) > > > > since %p4cc requires more bytes to hold the output characters. > > > > > > > > Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > > > --- > > > > .../interface/vchiq_arm/vchiq_arm.c | 20 +++++----- > > > > .../interface/vchiq_arm/vchiq_core.c | 40 +++++++++---------- > > > > .../interface/vchiq_arm/vchiq_core.h | 6 --- > > > > .../interface/vchiq_arm/vchiq_dev.c | 7 ++-- > > > > 4 files changed, 33 insertions(+), 40 deletions(-) > > > > > > > > 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 fc6d33ec5e95..de6a24304a4d 100644 > > > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > > > > @@ -1441,7 +1441,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, > > > > { > > > > struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); > > > > int ret = 0; > > > > - char entity[16]; > > > > + char entity[64]; > > > > int *entity_uc; > > > > int local_uc; > > > > @@ -1454,8 +1454,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, > > > > sprintf(entity, "VCHIQ: "); > > > > entity_uc = &arm_state->peer_use_count; > > > > } else if (service) { > > > > - sprintf(entity, "%c%c%c%c:%03d", > > > > - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), > > > > + sprintf(entity, "%p4cc:%03d", > > > Not related to your patch but these sprintfs() make me very > > > uncomfortable. > > > > > > KTODO: change sprintf() to snprintf() in staging/vc04_services/ > > Umang: how about one patch on top of this? :-) There are just five > > instances of it. > > Ricardo, how about this? Do you want to take a swing at this ? :-) > > And send a v2. ? No, that would be a separate change, this one is fine as-is. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes 2023-10-25 9:34 ` Umang Jain 2023-10-25 9:46 ` Greg Kroah-Hartman @ 2023-10-25 11:12 ` Ricardo B. Marliere 1 sibling, 0 replies; 8+ messages in thread From: Ricardo B. Marliere @ 2023-10-25 11:12 UTC (permalink / raw) To: Umang Jain Cc: Sakari Ailus, Dan Carpenter, linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel, Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham, Laurent Pinchart On 23/10/25 03:04PM, Umang Jain wrote: > > > On 10/25/23 2:58 PM, Sakari Ailus wrote: > > On Wed, Oct 25, 2023 at 10:10:08AM +0300, Dan Carpenter wrote: > >> On Wed, Oct 25, 2023 at 02:07:17AM -0400, Umang Jain wrote: > >>> Drop VCHIQ_FOURCC_AS_4CHARS macro in favour of %p4cc format > >>> modifier to print FourCC codes in the logs. > >>> > >>> vchiq_use_internal() and vchiq_release_internal() uses entity > >>> character-array to store a transient string that contains > >>> a FourCC code. Increase the length of entity array(to 64 bytes) > >>> since %p4cc requires more bytes to hold the output characters. > >>> > >>> Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > >>> --- > >>> .../interface/vchiq_arm/vchiq_arm.c | 20 +++++----- > >>> .../interface/vchiq_arm/vchiq_core.c | 40 +++++++++---------- > >>> .../interface/vchiq_arm/vchiq_core.h | 6 --- > >>> .../interface/vchiq_arm/vchiq_dev.c | 7 ++-- > >>> 4 files changed, 33 insertions(+), 40 deletions(-) > >>> > >>> 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 fc6d33ec5e95..de6a24304a4d 100644 > >>> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >>> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >>> @@ -1441,7 +1441,7 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, > >>> { > >>> struct vchiq_arm_state *arm_state = vchiq_platform_get_arm_state(state); > >>> int ret = 0; > >>> - char entity[16]; > >>> + char entity[64]; > >>> int *entity_uc; > >>> int local_uc; > >>> > >>> @@ -1454,8 +1454,8 @@ vchiq_use_internal(struct vchiq_state *state, struct vchiq_service *service, > >>> sprintf(entity, "VCHIQ: "); > >>> entity_uc = &arm_state->peer_use_count; > >>> } else if (service) { > >>> - sprintf(entity, "%c%c%c%c:%03d", > >>> - VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc), > >>> + sprintf(entity, "%p4cc:%03d", > >> Not related to your patch but these sprintfs() make me very > >> uncomfortable. > >> > >> KTODO: change sprintf() to snprintf() in staging/vc04_services/ > > Umang: how about one patch on top of this? :-) There are just five > > instances of it. > > Ricardo, how about this? Do you want to take a swing at this ? :-) Sure thing, I will send as another patch as Greg suggested. Thank you, - Ricardo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/1] staging: vc04_services: Use %p4cc to print fourcc 2023-10-25 6:07 [PATCH 0/1] staging: vc04_services: Use %p4cc to print fourcc Umang Jain 2023-10-25 6:07 ` [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes Umang Jain @ 2023-10-25 11:29 ` Stefan Wahren 1 sibling, 0 replies; 8+ messages in thread From: Stefan Wahren @ 2023-10-25 11:29 UTC (permalink / raw) To: Umang Jain, linux-staging, linux-rpi-kernel, linux-media, linux-arm-kernel, Phil Elwell, Dave Stevenson Cc: Stefan Wahren, Greg Kroah-Hartman, Dan Carpenter, Kieran Bingham, Laurent Pinchart, Ricardo B . Marliere, Sakari Ailus Hi, [add Raspberry Pi guys] Am 25.10.23 um 08:07 schrieb Umang Jain: > The following patch drop VCHIQ_FOURCC_AS_4CHARS macro in favour of %p4cc > format modifier to print FourCC codes in the logs. > > *Before this patch* > `mailbox: vchiq_core_msg debug: Sent Msg DATA(5) to AUDS s:1 d:62 len:20` > > *After this patch* > bcm2835_vchiq 3f00b840.mailbox: vchiq_core_msg debug: Sent Msg DATA(5) to SDUA little-endian (0x41554453) s:1 d:62 len:20 > > The inversion of AUDS to SDUA as per usage of %p4cc > Does it hamper readability ? Feedback is appreciated. looks good to me. Any concerns Phil or Dave? > > As documented in the commit message, the 'entity' char array length is > increased to hold more characters for the log output. Not doing so, > causes kernel stack corruption at runtime. > > Based on top of: > - [PATCH v2 0/8] staging: vc04: Drop custom logging based on printk > > Umang Jain (1): > staging: vc04_services: Use %p4cc format modifier to print FourCC > codes > > .../interface/vchiq_arm/vchiq_arm.c | 20 +++++----- > .../interface/vchiq_arm/vchiq_core.c | 40 +++++++++---------- > .../interface/vchiq_arm/vchiq_core.h | 6 --- > .../interface/vchiq_arm/vchiq_dev.c | 7 ++-- > 4 files changed, 33 insertions(+), 40 deletions(-) > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-10-25 11:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-25 6:07 [PATCH 0/1] staging: vc04_services: Use %p4cc to print fourcc Umang Jain 2023-10-25 6:07 ` [PATCH 1/1] staging: vc04_services: Use %p4cc format modifier to print FourCC codes Umang Jain 2023-10-25 7:10 ` Dan Carpenter 2023-10-25 9:28 ` Sakari Ailus 2023-10-25 9:34 ` Umang Jain 2023-10-25 9:46 ` Greg Kroah-Hartman 2023-10-25 11:12 ` Ricardo B. Marliere 2023-10-25 11:29 ` [PATCH 0/1] staging: vc04_services: Use %p4cc to print fourcc Stefan Wahren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox