Skip to content

Commit

Permalink
Replace memcpy on NameData with namestrcpy
Browse files Browse the repository at this point in the history
This PR replaces memcpy calls on NameData with namestrcpy. In addition,
a Coccinelle rule is introduced to detect these problems automatically.
  • Loading branch information
jnidzwetzki committed Feb 19, 2024
1 parent 57e6750 commit 677809d
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 63 deletions.
8 changes: 8 additions & 0 deletions coccinelle/namedata.cocci
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ symbol NAMEDATALEN;
+ /* You are using strlcpy with NAMEDATALEN, please consider using NameData and namestrcpy instead. */
+ namestrcpy(E1, E2);

@rule_namedata_memcpy@
expression E1, E2;
symbol NAMEDATALEN;
@@
- memcpy(E1, E2, NAMEDATALEN);
+ /* You are using memcpy with NAMEDATALEN, please consider using NameData and namestrcpy instead. */
+ namestrcpy(E1, E2);

@@
typedef NameData;
NameData E;
Expand Down
14 changes: 5 additions & 9 deletions src/bgw/job.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,7 @@ bgw_job_from_tupleinfo(TupleInfo *ti, size_t alloc_size)
job->fd.id = DatumGetInt32(values[AttrNumberGetAttrOffset(Anum_bgw_job_id)]);
if (!nulls[AttrNumberGetAttrOffset(Anum_bgw_job_application_name)])
namestrcpy(&job->fd.application_name,
NameStr(*DatumGetName(
values[AttrNumberGetAttrOffset(Anum_bgw_job_application_name)])));
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_bgw_job_application_name)]));
if (!nulls[AttrNumberGetAttrOffset(Anum_bgw_job_schedule_interval)])
job->fd.schedule_interval =
*DatumGetIntervalP(values[AttrNumberGetAttrOffset(Anum_bgw_job_schedule_interval)]);
Expand Down Expand Up @@ -279,19 +278,16 @@ bgw_job_from_tupleinfo(TupleInfo *ti, size_t alloc_size)

if (!nulls[AttrNumberGetAttrOffset(Anum_bgw_job_proc_schema)])
namestrcpy(&job->fd.proc_schema,
NameStr(
*DatumGetName(values[AttrNumberGetAttrOffset(Anum_bgw_job_proc_schema)])));
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_bgw_job_proc_schema)]));
if (!nulls[AttrNumberGetAttrOffset(Anum_bgw_job_proc_name)])
namestrcpy(&job->fd.proc_name,
NameStr(*DatumGetName(values[AttrNumberGetAttrOffset(Anum_bgw_job_proc_name)])));
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_bgw_job_proc_name)]));
if (!nulls[AttrNumberGetAttrOffset(Anum_bgw_job_check_schema)])
namestrcpy(&job->fd.check_schema,
NameStr(
*DatumGetName(values[AttrNumberGetAttrOffset(Anum_bgw_job_check_schema)])));
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_bgw_job_check_schema)]));
if (!nulls[AttrNumberGetAttrOffset(Anum_bgw_job_check_name)])
namestrcpy(&job->fd.check_name,
NameStr(
*DatumGetName(values[AttrNumberGetAttrOffset(Anum_bgw_job_check_name)])));
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_bgw_job_check_name)]));
if (!nulls[AttrNumberGetAttrOffset(Anum_bgw_job_owner)])
job->fd.owner = DatumGetObjectId(values[AttrNumberGetAttrOffset(Anum_bgw_job_owner)]);

Expand Down
10 changes: 4 additions & 6 deletions src/chunk.c
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,10 @@ ts_chunk_formdata_fill(FormData_chunk *fd, const TupleInfo *ti)

fd->id = DatumGetInt32(values[AttrNumberGetAttrOffset(Anum_chunk_id)]);
fd->hypertable_id = DatumGetInt32(values[AttrNumberGetAttrOffset(Anum_chunk_hypertable_id)]);
memcpy(&fd->schema_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_chunk_schema_name)]),
NAMEDATALEN);
memcpy(&fd->table_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_chunk_table_name)]),
NAMEDATALEN);
namestrcpy(&fd->schema_name,
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_chunk_schema_name)]));
namestrcpy(&fd->table_name,
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_chunk_table_name)]));

if (nulls[AttrNumberGetAttrOffset(Anum_chunk_compressed_chunk_id)])
fd->compressed_chunk_id = INVALID_CHUNK_ID;
Expand Down
20 changes: 9 additions & 11 deletions src/dimension.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,8 @@ dimension_fill_in_from_tuple(Dimension *d, TupleInfo *ti, Oid main_table_relid)
d->fd.aligned = DatumGetBool(values[AttrNumberGetAttrOffset(Anum_dimension_aligned)]);
d->fd.column_type =
DatumGetObjectId(values[AttrNumberGetAttrOffset(Anum_dimension_column_type)]);
memcpy(&d->fd.column_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_dimension_column_name)]),
NAMEDATALEN);
namestrcpy(&d->fd.column_name,
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_dimension_column_name)]));

if (!isnull[AttrNumberGetAttrOffset(Anum_dimension_partitioning_func_schema)] &&
!isnull[AttrNumberGetAttrOffset(Anum_dimension_partitioning_func)])
Expand All @@ -200,13 +199,12 @@ dimension_fill_in_from_tuple(Dimension *d, TupleInfo *ti, Oid main_table_relid)
d->fd.num_slices =
DatumGetInt16(values[AttrNumberGetAttrOffset(Anum_dimension_num_slices)]);

memcpy(&d->fd.partitioning_func_schema,
DatumGetName(
values[AttrNumberGetAttrOffset(Anum_dimension_partitioning_func_schema)]),
NAMEDATALEN);
memcpy(&d->fd.partitioning_func,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_dimension_partitioning_func)]),
NAMEDATALEN);
namestrcpy(&d->fd.partitioning_func_schema,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_dimension_partitioning_func_schema)]));
namestrcpy(&d->fd.partitioning_func,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_dimension_partitioning_func)]));

old = MemoryContextSwitchTo(ti->mctx);
d->partitioning = ts_partitioning_info_create(NameStr(d->fd.partitioning_func_schema),
Expand Down Expand Up @@ -1680,7 +1678,7 @@ ts_dimension_add(PG_FUNCTION_ARGS)
TS_PREVENT_FUNC_IF_READ_ONLY();

if (!PG_ARGISNULL(1))
memcpy(&info.colname, PG_GETARG_NAME(1), NAMEDATALEN);
namestrcpy(&info.colname, NameStr(*PG_GETARG_NAME(1)));

if (PG_ARGISNULL(0))
ereport(ERROR,
Expand Down
34 changes: 16 additions & 18 deletions src/hypertable.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,28 +196,26 @@ ts_hypertable_formdata_fill(FormData_hypertable *fd, const TupleInfo *ti)
Assert(!nulls[AttrNumberGetAttrOffset(Anum_hypertable_status)]);

fd->id = DatumGetInt32(values[AttrNumberGetAttrOffset(Anum_hypertable_id)]);
memcpy(&fd->schema_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_hypertable_schema_name)]),
NAMEDATALEN);
memcpy(&fd->table_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_hypertable_table_name)]),
NAMEDATALEN);
memcpy(&fd->associated_schema_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_hypertable_associated_schema_name)]),
NAMEDATALEN);
memcpy(&fd->associated_table_prefix,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_hypertable_associated_table_prefix)]),
NAMEDATALEN);
namestrcpy(&fd->schema_name,
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_hypertable_schema_name)]));
namestrcpy(&fd->table_name,
DatumGetCString(values[AttrNumberGetAttrOffset(Anum_hypertable_table_name)]));
namestrcpy(&fd->associated_schema_name,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_hypertable_associated_schema_name)]));
namestrcpy(&fd->associated_table_prefix,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_hypertable_associated_table_prefix)]));

fd->num_dimensions =
DatumGetInt16(values[AttrNumberGetAttrOffset(Anum_hypertable_num_dimensions)]);

memcpy(&fd->chunk_sizing_func_schema,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_hypertable_chunk_sizing_func_schema)]),
NAMEDATALEN);
memcpy(&fd->chunk_sizing_func_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_hypertable_chunk_sizing_func_name)]),
NAMEDATALEN);
namestrcpy(&fd->chunk_sizing_func_schema,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_hypertable_chunk_sizing_func_schema)]));
namestrcpy(&fd->chunk_sizing_func_name,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_hypertable_chunk_sizing_func_name)]));

fd->chunk_target_size =
DatumGetInt64(values[AttrNumberGetAttrOffset(Anum_hypertable_chunk_target_size)]);
Expand Down
38 changes: 19 additions & 19 deletions src/ts_catalog/continuous_agg.c
Original file line number Diff line number Diff line change
Expand Up @@ -369,29 +369,29 @@ continuous_agg_formdata_fill(FormData_continuous_agg *fd, const TupleInfo *ti)
fd->parent_mat_hypertable_id = DatumGetInt32(
values[AttrNumberGetAttrOffset(Anum_continuous_agg_parent_mat_hypertable_id)]);

memcpy(&fd->user_view_schema,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_continuous_agg_user_view_schema)]),
NAMEDATALEN);
memcpy(&fd->user_view_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_continuous_agg_user_view_name)]),
NAMEDATALEN);

memcpy(&fd->partial_view_schema,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_continuous_agg_partial_view_schema)]),
NAMEDATALEN);
memcpy(&fd->partial_view_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_continuous_agg_partial_view_name)]),
NAMEDATALEN);
namestrcpy(&fd->user_view_schema,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_continuous_agg_user_view_schema)]));
namestrcpy(&fd->user_view_name,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_continuous_agg_user_view_name)]));

namestrcpy(&fd->partial_view_schema,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_continuous_agg_partial_view_schema)]));
namestrcpy(&fd->partial_view_name,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_continuous_agg_partial_view_name)]));

fd->bucket_width =
DatumGetInt64(values[AttrNumberGetAttrOffset(Anum_continuous_agg_bucket_width)]);

memcpy(&fd->direct_view_schema,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_continuous_agg_direct_view_schema)]),
NAMEDATALEN);
memcpy(&fd->direct_view_name,
DatumGetName(values[AttrNumberGetAttrOffset(Anum_continuous_agg_direct_view_name)]),
NAMEDATALEN);
namestrcpy(&fd->direct_view_schema,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_continuous_agg_direct_view_schema)]));
namestrcpy(&fd->direct_view_name,
DatumGetCString(
values[AttrNumberGetAttrOffset(Anum_continuous_agg_direct_view_name)]));

fd->materialized_only =
DatumGetBool(values[AttrNumberGetAttrOffset(Anum_continuous_agg_materialize_only)]);
Expand Down

0 comments on commit 677809d

Please sign in to comment.