Added coccinelle rule to find strlcpy on NameData

[jnidzwetzki]

NameData is a fixed-size type of 64 bytes. Using strlcpy to copy data into a NameData struct can cause problems because any data that follows the initial null-terminated string will also be part of the data.

Note:
Should be merged after #5336, #5345, and #5317

Disable-check: force-changelog-changed

[codecov]

Codecov Report

Merging #5340 (46a6ada) into main (8a51a76) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #5340      +/-   ##
==========================================
- Coverage   90.68%   90.67%   -0.01%     
==========================================
  Files         225      225              
  Lines       45042    52016    +6974     
==========================================
+ Hits        40848    47168    +6320     
- Misses       4194     4848     +654     

Impacted Files	Coverage Δ
src/nodes/hypertable_modify.c	71.19% <0.00%> (-24.02%)	⬇️
src/uuid.c	78.57% <0.00%> (-6.05%)	⬇️
tsl/src/remote/healthcheck.c	63.76% <0.00%> (-6.05%)	⬇️
src/planner/add_hashagg.c	48.95% <0.00%> (-5.46%)	⬇️
src/histogram.c	83.83% <0.00%> (-4.80%)	⬇️
src/extension_utils.c	90.74% <0.00%> (-4.72%)	⬇️
tsl/src/fdw/fdw_utils.c	80.76% <0.00%> (-4.65%)	⬇️
src/time_utils.c	93.72% <0.00%> (-3.78%)	⬇️
src/utils.h	80.00% <0.00%> (-3.34%)	⬇️
tsl/src/remote/copy_fetcher.c	85.19% <0.00%> (-3.02%)	⬇️
... and 192 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[mkindahl]

Not sure if you should have the & there: the expressions are already pointers, so if you take the address of a pointer this will not work in most cases.

[jnidzwetzki]

My assumption was that the variable is also changed from char[] to NameData. In this case, the & operator is required here. However, since the change of the variable type is not part of this rule and it depends on the context, I have removed the suggestion and only annotate the strlcpy call.

[kgyrtkirk]

I think after this change most likely E1 in its original will not work anymore...the rule does drag attention to it - and I think that's what it should do!

[mkindahl]

My assumption was that the variable is also changed from char[] to NameData. In this case, the & operator is required here.

Exactly.

However, since the change of the variable type is not part of this rule and it depends on the context, I have removed the suggestion and only annotate the strlcpy call.

Well. Given that the NAMEDATALEN is part of the rule, I think it quite clear that the target is a pointer to a buffer with this length, so it makes sense to still suggest it as a transformation. Especially considering the fact that it is a strlcpy. It is quite common to use memcpy as well, but this will copy all the data and does not suffer from this problem.

I actually think that you can add the "struct" rule that you used before. It is quite clearly the case that when you store something like this in a structure, you should use NameData. Local variables is a different story.

[jnidzwetzki]

Good points. I added the suggestion of the transformation for strlcpy and the rule for struct members. In addition, I enabled Coccinelle checks for *.h files in the newest version of this PR.