Skip to content

video: driver: copy struct v4l2_format by assignment#125

Open
ricardosalveti wants to merge 1 commit into
qualcomm-linux:video.qclinux.mainfrom
ricardosalveti:fix-v4l2-format-stringop-overread
Open

video: driver: copy struct v4l2_format by assignment#125
ricardosalveti wants to merge 1 commit into
qualcomm-linux:video.qclinux.mainfrom
ricardosalveti:fix-v4l2-format-stringop-overread

Conversation

@ricardosalveti

Copy link
Copy Markdown

GCC 16.1 with -Wstringop-overread (turned into an error by the driver's own -Werror) rejects the byte-wise copies of a whole struct v4l2_format:

msm_venc.c:1327:9: error: 'memcpy' reading 208 bytes from a region
of size 0 [-Werror=stringop-overread]
note: source object is likely at address zero

After msm_venc_s_fmt_input() is inlined into msm_venc_s_fmt(), the compiler's value-range analysis mis-judges the source object (&inst->fmts[INPUT_PORT]) as a zero-sized region at address zero and rejects the memcpy(). The source is a regular member of struct msm_vidc_inst and is never NULL, so this is a false positive, but it breaks the build for newer toolchains.

Copy the structures with a plain aggregate assignment instead of memcpy(). The assignment is a by-value copy of the whole structure - it copies the entire object, including the embedded union, exactly as the old memcpy(..., sizeof(struct v4l2_format)) did. It lets the compiler derive the size from the type and is not subject to the string/memory builtin diagnostics, so it both fixes the build and is more idiomatic.

No functional change intended.

Assisted-by: Claude:claude-opus-4.8

GCC 16.1 with -Wstringop-overread (turned into an error by the
driver's own -Werror) rejects the byte-wise copies of a whole
struct v4l2_format:

  msm_venc.c:1327:9: error: 'memcpy' reading 208 bytes from a region
  of size 0 [-Werror=stringop-overread]
  note: source object is likely at address zero

After msm_venc_s_fmt_input() is inlined into msm_venc_s_fmt(), the
compiler's value-range analysis mis-judges the source object
(&inst->fmts[INPUT_PORT]) as a zero-sized region at address zero and
rejects the memcpy(). The source is a regular member of struct
msm_vidc_inst and is never NULL, so this is a false positive, but it
breaks the build for newer toolchains.

Copy the structures with a plain aggregate assignment instead of
memcpy(). The assignment is a by-value copy of the whole structure -
it copies the entire object, including the embedded union, exactly as
the old memcpy(..., sizeof(struct v4l2_format)) did. It lets the
compiler derive the size from the type and is not subject to the
string/memory builtin diagnostics, so it both fixes the build and is
more idiomatic.

No functional change intended.

Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Ricardo Salveti <ricardo.salveti@oss.qualcomm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant