All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Bommu Krishnaiah <krishnaiah.bommu@intel.com>,
	intel-xe@lists.freedesktop.org
Cc: Bommu Krishnaiah <krishnaiah.bommu@intel.com>,
	Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>,
	Tejas Upadhyay <tejas.upadhyay@intel.com>
Subject: Re: [PATCH v2] drm/xe : avoid the risky function sprintf
Date: Fri, 22 Mar 2024 11:40:11 +0200	[thread overview]
Message-ID: <87plvmli4k.fsf@intel.com> (raw)
In-Reply-To: <20231206214903.46065-1-krishnaiah.bommu@intel.com>


drm/xe : avoid the risky function sprintf
      ^

Please don't add a space there in the subject.

On Thu, 07 Dec 2023, Bommu Krishnaiah <krishnaiah.bommu@intel.com> wrote:
> These errors were highlighted by Static analyzer. Used snprintf instead of sprint.
>
> Static analyzer Reported "Calling risky function (DC.STRING_BUFFER)”
>
> v2: Removed hard coded values abd used sizeof()
>
> Signed-off-by: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_debugfs.c               |  2 +-
>  drivers/gpu/drm/xe/xe_exec_queue.c            | 12 +++----
>  drivers/gpu/drm/xe/xe_gt_debugfs.c            |  2 +-
>  drivers/gpu/drm/xe/xe_gt_idle.c               |  4 +--
>  drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c | 36 +++++++++----------
>  drivers/gpu/drm/xe/xe_hw_fence.c              |  2 +-
>  6 files changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c
> index 8abdf3c17e1d..70532d67eb99 100644
> --- a/drivers/gpu/drm/xe/xe_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_debugfs.c
> @@ -129,7 +129,7 @@ void xe_debugfs_register(struct xe_device *xe)
>  		if (man) {
>  			char name[16];
>  
> -			sprintf(name, "vram%d_mm", mem_type - XE_PL_VRAM0);
> +			snprintf(name, 16, "vram%d_mm", mem_type - XE_PL_VRAM0);

Please never use magic values like this when you can use
e.g. sizeof(name).

>  			ttm_resource_manager_create_debugfs(man, root, name);
>  		}
>  	}
> diff --git a/drivers/gpu/drm/xe/xe_exec_queue.c b/drivers/gpu/drm/xe/xe_exec_queue.c
> index 730eb7d2a639..14d6c1d6e434 100644
> --- a/drivers/gpu/drm/xe/xe_exec_queue.c
> +++ b/drivers/gpu/drm/xe/xe_exec_queue.c
> @@ -225,22 +225,22 @@ void xe_exec_queue_assign_name(struct xe_exec_queue *q, u32 instance)
>  {
>  	switch (q->class) {
>  	case XE_ENGINE_CLASS_RENDER:
> -		sprintf(q->name, "rcs%d", instance);
> +		snprintf(q->name, MAX_FENCE_NAME_LEN, "rcs%d", instance);

Like Himal suggested, please use sizeof.

>  		break;
>  	case XE_ENGINE_CLASS_VIDEO_DECODE:
> -		sprintf(q->name, "vcs%d", instance);
> +		snprintf(q->name, MAX_FENCE_NAME_LEN, "vcs%d", instance);
>  		break;
>  	case XE_ENGINE_CLASS_VIDEO_ENHANCE:
> -		sprintf(q->name, "vecs%d", instance);
> +		snprintf(q->name, MAX_FENCE_NAME_LEN, "vecs%d", instance);
>  		break;
>  	case XE_ENGINE_CLASS_COPY:
> -		sprintf(q->name, "bcs%d", instance);
> +		snprintf(q->name, MAX_FENCE_NAME_LEN, "bcs%d", instance);
>  		break;
>  	case XE_ENGINE_CLASS_COMPUTE:
> -		sprintf(q->name, "ccs%d", instance);
> +		snprintf(q->name, MAX_FENCE_NAME_LEN, "ccs%d", instance);
>  		break;
>  	case XE_ENGINE_CLASS_OTHER:
> -		sprintf(q->name, "gsccs%d", instance);
> +		snprintf(q->name, MAX_FENCE_NAME_LEN, "gsccs%d", instance);
>  		break;
>  	default:
>  		XE_WARN_ON(q->class);
> diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> index 6b4dc2927727..3af85b2f6625 100644
> --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c
> @@ -261,7 +261,7 @@ void xe_gt_debugfs_register(struct xe_gt *gt)
>  
>  	xe_gt_assert(gt, minor->debugfs_root);
>  
> -	sprintf(name, "gt%d", gt->info.id);
> +	snprintf(name, sizeof(name), "gt%d", gt->info.id);
>  	root = debugfs_create_dir(name, minor->debugfs_root);
>  	if (IS_ERR(root)) {
>  		drm_warn(&xe->drm, "Create GT directory failed");
> diff --git a/drivers/gpu/drm/xe/xe_gt_idle.c b/drivers/gpu/drm/xe/xe_gt_idle.c
> index 2984680de3f9..bc1426f8d731 100644
> --- a/drivers/gpu/drm/xe/xe_gt_idle.c
> +++ b/drivers/gpu/drm/xe/xe_gt_idle.c
> @@ -166,10 +166,10 @@ void xe_gt_idle_sysfs_init(struct xe_gt_idle *gtidle)
>  	}
>  
>  	if (xe_gt_is_media_type(gt)) {
> -		sprintf(gtidle->name, "gt%d-mc", gt->info.id);
> +		snprintf(gtidle->name, sizeof(gtidle->name), "gt%d-mc", gt->info.id);
>  		gtidle->idle_residency = xe_guc_pc_mc6_residency;
>  	} else {
> -		sprintf(gtidle->name, "gt%d-rc", gt->info.id);
> +		snprintf(gtidle->name, sizeof(gtidle->name), "gt%d-rc", gt->info.id);
>  		gtidle->idle_residency = xe_guc_pc_rc6_residency;
>  	}
>  
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> index c17cce53f19d..f85a40d87829 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine_class_sysfs.c
> @@ -71,7 +71,7 @@ static ssize_t job_timeout_max_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.job_timeout_max);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.job_timeout_max);
>  }
>  
>  static const struct kobj_attribute job_timeout_max_attr =
> @@ -107,7 +107,7 @@ static ssize_t job_timeout_min_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.job_timeout_min);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.job_timeout_min);
>  }
>  
>  static const struct kobj_attribute job_timeout_min_attr =
> @@ -140,7 +140,7 @@ static ssize_t job_timeout_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.job_timeout_ms);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.job_timeout_ms);
>  }
>  
>  static const struct kobj_attribute job_timeout_attr =
> @@ -151,7 +151,7 @@ static ssize_t job_timeout_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.job_timeout_ms);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.job_timeout_ms);
>  }
>  
>  static const struct kobj_attribute job_timeout_def =
> @@ -162,7 +162,7 @@ static ssize_t job_timeout_min_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.job_timeout_min);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.job_timeout_min);
>  }
>  
>  static const struct kobj_attribute job_timeout_min_def =
> @@ -173,7 +173,7 @@ static ssize_t job_timeout_max_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.job_timeout_max);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.job_timeout_max);
>  }
>  
>  static const struct kobj_attribute job_timeout_max_def =
> @@ -232,7 +232,7 @@ static ssize_t timeslice_duration_max_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.timeslice_max);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.timeslice_max);
>  }
>  
>  static const struct kobj_attribute timeslice_duration_max_attr =
> @@ -270,7 +270,7 @@ static ssize_t timeslice_duration_min_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.timeslice_min);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.timeslice_min);
>  }
>  
>  static const struct kobj_attribute timeslice_duration_min_attr =
> @@ -282,7 +282,7 @@ static ssize_t timeslice_duration_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.timeslice_us);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.timeslice_us);
>  }
>  
>  static const struct kobj_attribute timeslice_duration_attr =
> @@ -294,7 +294,7 @@ static ssize_t timeslice_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.timeslice_us);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.timeslice_us);
>  }
>  
>  static const struct kobj_attribute timeslice_duration_def =
> @@ -305,7 +305,7 @@ static ssize_t timeslice_min_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.timeslice_min);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.timeslice_min);
>  }
>  
>  static const struct kobj_attribute timeslice_duration_min_def =
> @@ -316,7 +316,7 @@ static ssize_t timeslice_max_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.timeslice_max);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.timeslice_max);
>  }
>  
>  static const struct kobj_attribute timeslice_duration_max_def =
> @@ -349,7 +349,7 @@ static ssize_t preempt_timeout_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.preempt_timeout_us);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.preempt_timeout_us);
>  }
>  
>  static const struct kobj_attribute preempt_timeout_attr =
> @@ -361,7 +361,7 @@ static ssize_t preempt_timeout_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.preempt_timeout_us);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.preempt_timeout_us);
>  }
>  
>  static const struct kobj_attribute preempt_timeout_def =
> @@ -373,7 +373,7 @@ static ssize_t preempt_timeout_min_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.preempt_timeout_min);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.preempt_timeout_min);
>  }
>  
>  static const struct kobj_attribute preempt_timeout_min_def =
> @@ -385,7 +385,7 @@ static ssize_t preempt_timeout_max_default(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj->parent);
>  
> -	return sprintf(buf, "%u\n", eclass->defaults.preempt_timeout_max);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->defaults.preempt_timeout_max);
>  }
>  
>  static const struct kobj_attribute preempt_timeout_max_def =
> @@ -421,7 +421,7 @@ static ssize_t preempt_timeout_max_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.preempt_timeout_max);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.preempt_timeout_max);
>  }
>  
>  static const struct kobj_attribute preempt_timeout_max_attr =
> @@ -458,7 +458,7 @@ static ssize_t preempt_timeout_min_show(struct kobject *kobj,
>  {
>  	struct xe_hw_engine_class_intf *eclass = kobj_to_eclass(kobj);
>  
> -	return sprintf(buf, "%u\n", eclass->sched_props.preempt_timeout_min);
> +	return snprintf(buf, PAGE_SIZE, "%u\n", eclass->sched_props.preempt_timeout_min);
>  }
>  
>  static const struct kobj_attribute preempt_timeout_min_attr =
> diff --git a/drivers/gpu/drm/xe/xe_hw_fence.c b/drivers/gpu/drm/xe/xe_hw_fence.c
> index a5de3e7b0bd6..91245002d965 100644
> --- a/drivers/gpu/drm/xe/xe_hw_fence.c
> +++ b/drivers/gpu/drm/xe/xe_hw_fence.c
> @@ -130,7 +130,7 @@ void xe_hw_fence_ctx_init(struct xe_hw_fence_ctx *ctx, struct xe_gt *gt,
>  	ctx->irq = irq;
>  	ctx->dma_fence_ctx = dma_fence_context_alloc(1);
>  	ctx->next_seqno = XE_FENCE_INITIAL_SEQNO;
> -	sprintf(ctx->name, "%s", name);
> +	snprintf(ctx->name, MAX_FENCE_NAME_LEN, "%s", name);

Again, sizeof.

>  }
>  
>  void xe_hw_fence_ctx_finish(struct xe_hw_fence_ctx *ctx)

-- 
Jani Nikula, Intel

      parent reply	other threads:[~2024-03-22  9:40 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 21:49 [PATCH v2] drm/xe : avoid the risky function sprintf Bommu Krishnaiah
2024-03-22  6:27 ` Ghimiray, Himal Prasad
2024-03-22  7:38 ` ✓ CI.Patch_applied: success for drm/xe : avoid the risky function sprintf (rev2) Patchwork
2024-03-22  7:38 ` ✗ CI.checkpatch: warning " Patchwork
2024-03-22  7:39 ` ✓ CI.KUnit: success " Patchwork
2024-03-22  7:50 ` ✓ CI.Build: " Patchwork
2024-03-22  7:53 ` ✓ CI.Hooks: " Patchwork
2024-03-22  7:58 ` ✓ CI.checksparse: " Patchwork
2024-03-22  8:24 ` ✓ CI.BAT: " Patchwork
2024-03-22  9:40 ` Jani Nikula [this message]

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=87plvmli4k.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=krishnaiah.bommu@intel.com \
    --cc=tejas.upadhyay@intel.com \
    /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.