Re: [Cocci] git-coccinelle: adjustments for array.cocci?
>> + memcpy( >> +( ptr, E, n * >> +- sizeof(*(ptr)) >> ++ sizeof(T) >> +| arr, E, n * >> +- sizeof(*(arr)) >> ++ sizeof(T) >> +| E, ptr, n * >> +- sizeof(*(ptr)) >> ++ sizeof(T) >> +| E, arr, n * >> +- sizeof(*(arr)) >> ++ sizeof(T) >> ) >> + ) > > This seems quite unreadable, in contrast to the original code. The code formatting can vary for improved applications of SmPL disjunctions. See also related update suggestions: * https://public-inbox.org/git/05ab1110-2115-7886-f890-9983caabc...@web.de/ * https://public-inbox.org/git/75b9417b-14a7-c9c6-25eb-f6e05f340...@web.de/ >> 5. I stored another generated patch based on the adjusted SmPL script. > > No idea what it means to store a patch. I put the output from the program “spatch” into a text file like “array-reduced1.diff” in a selected directory. >> 6. I performed a corresponding file comparison. >> >> --- array-released.diff 2019-11-14 21:29:11.020576916 +0100 >> +++ array-reduced1.diff 2019-11-14 21:45:58.931956527 +0100 >> @@ -6,24 +6,10 @@ >> r->entry_count = t->entry_count; >> r->delta_depth = t->delta_depth; >> - memcpy(r->entries,t->entries,t->entry_count*sizeof(t->entries[0])); >> -+ COPY_ARRAY(r->entries, t->entries, t->entry_count); >> ++ memcpy(r->entries,t->entries,t->entry_count*sizeof(*(t->entries))); >> release_tree_content(t); >> return r; >> } > > I have no idea what is being compared here. I suggest to take another look at the described steps then. > The COPY_ARRAY thing looks nice, but doesn't seem to have anything to do > with your semantic patch. I find your interpretation of the presented software situation questionable. * I got the impression in the meantime that my suggestion for a refactoring of a specific SmPL disjunction influenced transformation results for a subsequent SmPL rule in unexpected ways. * Other software adjustments and solution variants can trigger further development considerations, can't they? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] git-coccinelle: adjustments for array.cocci?
> It can determine the type of t->entries if it has access to the definition > of the type of t. I would like to point another implementation detail out. Another known function was also an update candidate. https://github.com/git/git/blob/9a1180fc304ad9831641e5788e9c8d3dfc10ccdd/pretty.c#L90 elfring@Sonne:~/Projekte/git/lokal> spatch contrib/coccinelle/array.cocci pretty.c … @@ -106,8 +106,8 @@ static void setup_commit_formats(void) commit_formats_len = ARRAY_SIZE(builtin_formats); builtin_formats_len = commit_formats_len; ALLOC_GROW(commit_formats, commit_formats_len, commit_formats_alloc); - memcpy(commit_formats, builtin_formats, - sizeof(*builtin_formats)*ARRAY_SIZE(builtin_formats)); + COPY_ARRAY(commit_formats, builtin_formats, + ARRAY_SIZE(builtin_formats)); git_config(git_pretty_formats_config, NULL); } This patch generation can work based on the following SmPL code combination. “… expression n, x; … - , (n) * \( sizeof(T) \| sizeof(*(x)) \) …” The asterisk should refer to a pointer expression within a sizeof operator. I got informed that the semantic patch language would support such a restriction. Thus I have tried out to specify the corresponding metavariables in this way. “… expression n; expression* x; …” But the shown diff hunk is not regenerated by this SmPL script variant. How should an array like “builtin_formats” (which is even defined in the same function) be treated by the Coccinelle software in such use cases? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] git-coccinelle: adjustments for array.cocci?
>> https://github.com/git/git/blob/3edfcc65fdfc708c1c8f1d314885eecf9beb9b67/fast-import.c#L640 >> >> I got the impression that the Coccinelle software is occasionally able >> to determine from the search specification “sizeof(T)” the corresponding >> data type for code like “*(t->entries)”. > > It can determine the type of t->entries if it has access to the definition > of the type of t. Should this type determination always work here because the data structure “tree_content” for the parameter “t” of the function “grow_tree_content” is defined in the same source file? https://github.com/git/git/blob/3edfcc65fdfc708c1c8f1d314885eecf9beb9b67/fast-import.c#L85 >This type may be in a header file. If you want > Coccinelle to be able to find this information you can use the option > --all-includes or --recursive-includes. It will be more efficient with > the option --include-headers-for-types. Such information can be more helpful in other situations than the mentioned test case. >> But it seems that there are circumstances to consider where the desired >> data type was not automatically determined. Would you like to take the presented differences from the discussed before/after comparison better into account? Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] git-coccinelle: adjustments for array.cocci?
On Fri, 15 Nov 2019, Markus Elfring wrote: > > --- array-released.diff 2019-11-14 21:29:11.020576916 +0100 > > +++ array-reduced1.diff 2019-11-14 21:45:58.931956527 +0100 > > @@ -6,24 +6,10 @@ > > r->entry_count = t->entry_count; > > r->delta_depth = t->delta_depth; > > - memcpy(r->entries,t->entries,t->entry_count*sizeof(t->entries[0])); > > -+ COPY_ARRAY(r->entries, t->entries, t->entry_count); > > ++ memcpy(r->entries,t->entries,t->entry_count*sizeof(*(t->entries))); > > release_tree_content(t); > > return r; > > } > > It took a while to become more aware of software development challenges > for the safe data processing with the semantic patch language also > at such a source code place. > https://github.com/git/git/blob/3edfcc65fdfc708c1c8f1d314885eecf9beb9b67/fast-import.c#L640 > > I got the impression that the Coccinelle software is occasionally able > to determine from the search specification “sizeof(T)” the corresponding > data type for code like “*(t->entries)”. It can determine the type of t->entries if it has access to the definition of the type of t. This type may be in a header file. If you want Coccinelle to be able to find this information you can use the option --all-includes or --recursive-includes. It will be more efficient with the option --include-headers-for-types. julia > But it seems that there are circumstances to consider where the desired > data type was not automatically determined. > Thus the data processing can become safer by explicitly expressing > the case distinction for the handling of expressions. > > Adjusted transformation rule: > @@ > type T; > T* dst_ptr, src_ptr; > T[] dst_arr, src_arr; > expression n, x; > @@ > -memcpy > +COPY_ARRAY >( > ( dst_ptr > | dst_arr > ) >, > ( src_ptr > | src_arr > ) >, > - (n) * \( sizeof(T) \| sizeof(*(x)) \) > + n >) > > > Regards, > Markus > ___ > Cocci mailing list > Cocci@systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci >___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] git-coccinelle: adjustments for array.cocci?
> --- array-released.diff 2019-11-14 21:29:11.020576916 +0100 > +++ array-reduced1.diff 2019-11-14 21:45:58.931956527 +0100 > @@ -6,24 +6,10 @@ > r->entry_count = t->entry_count; > r->delta_depth = t->delta_depth; > -memcpy(r->entries,t->entries,t->entry_count*sizeof(t->entries[0])); > -+COPY_ARRAY(r->entries, t->entries, t->entry_count); > ++memcpy(r->entries,t->entries,t->entry_count*sizeof(*(t->entries))); > release_tree_content(t); > return r; > } It took a while to become more aware of software development challenges for the safe data processing with the semantic patch language also at such a source code place. https://github.com/git/git/blob/3edfcc65fdfc708c1c8f1d314885eecf9beb9b67/fast-import.c#L640 I got the impression that the Coccinelle software is occasionally able to determine from the search specification “sizeof(T)” the corresponding data type for code like “*(t->entries)”. But it seems that there are circumstances to consider where the desired data type was not automatically determined. Thus the data processing can become safer by explicitly expressing the case distinction for the handling of expressions. Adjusted transformation rule: @@ type T; T* dst_ptr, src_ptr; T[] dst_arr, src_arr; expression n, x; @@ -memcpy +COPY_ARRAY ( ( dst_ptr | dst_arr ) , ( src_ptr | src_arr ) , - (n) * \( sizeof(T) \| sizeof(*(x)) \) + n ) Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] git-coccinelle: adjustments for array.cocci?
> --- array-released.diff 2019-11-14 21:29:11.020576916 +0100 > +++ array-reduced1.diff 2019-11-14 21:45:58.931956527 +0100 > @@ -6,24 +6,10 @@ > r->entry_count = t->entry_count; > r->delta_depth = t->delta_depth; > -memcpy(r->entries,t->entries,t->entry_count*sizeof(t->entries[0])); > -+COPY_ARRAY(r->entries, t->entries, t->entry_count); > ++memcpy(r->entries,t->entries,t->entry_count*sizeof(*(t->entries))); > release_tree_content(t); > return r; > } Can another variant for a transformation rule help to clarify unexpected software behaviour around data processing with the semantic patch language? @@ expression dst, src, n, E; type T; T *ptr; T[] arr; @@ memcpy( (dst, src, sizeof( + *( E -[...] + ) ) * n | ptr, src, sizeof( -*(ptr) +T ) * n | arr, src, sizeof( -*(arr) +T ) * n | dst, ptr, sizeof( -*(ptr) +T ) * n | dst, arr, sizeof( -*(arr) +T ) * n ) ) elfring@Sonne:~/Projekte/git/lokal> spatch contrib/coccinelle/array-test3.cocci fast-import.c … Regards, Markus ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] git-coccinelle: adjustments for array.cocci?
> Anyway, someone who can reproduce the issue using the latest release
> of Coccinelle would be in a better position to file a bug report.
Hello,
I repeated the discussed source code transformation approach together
with the software combination “Coccinelle 1.0.8-4-g842075f7” (OCaml 4.09).
https://github.com/coccinelle/coccinelle/commits/master
1. Yesterday I checked the source files out for the software “Git”
according to the commit “The first batch post 2.24 cycle”.
https://github.com/git/git/commit/d9f6f3b6195a0ca35642561e530798ad1469bd41
2. I restored a previous development status by the following command.
git show 921d49be86 | patch -p1 -R
See also:
https://public-inbox.org/git/53346d52-e096-c651-f70a-ce6ca4d82...@web.de/
3. I stored a generated patch based on the currently released SmPL script.
https://github.com/git/git/blob/177fbab747da4f58cb2a8ce010b3515c86dd67c9/contrib/coccinelle/array.cocci
4. I applied the following patch then.
diff --git a/contrib/coccinelle/array.cocci b/contrib/coccinelle/array.cocci
index 46b8d2ee11..89df184bbd 100644
--- a/contrib/coccinelle/array.cocci
+++ b/contrib/coccinelle/array.cocci
@@ -12,27 +12,21 @@ T *ptr;
T[] arr;
expression E, n;
@@
-(
- memcpy(ptr, E,
-- n * sizeof(*(ptr))
-+ n * sizeof(T)
- )
-|
- memcpy(arr, E,
-- n * sizeof(*(arr))
-+ n * sizeof(T)
- )
-|
- memcpy(E, ptr,
-- n * sizeof(*(ptr))
-+ n * sizeof(T)
- )
-|
- memcpy(E, arr,
-- n * sizeof(*(arr))
-+ n * sizeof(T)
- )
+ memcpy(
+( ptr, E, n *
+- sizeof(*(ptr))
++ sizeof(T)
+| arr, E, n *
+- sizeof(*(arr))
++ sizeof(T)
+| E, ptr, n *
+- sizeof(*(ptr))
++ sizeof(T)
+| E, arr, n *
+- sizeof(*(arr))
++ sizeof(T)
)
+ )
@@
type T;
I suggested in this way to move a bit of SmPL code.
5. I stored another generated patch based on the adjusted SmPL script.
6. I performed a corresponding file comparison.
--- array-released.diff 2019-11-14 21:29:11.020576916 +0100
+++ array-reduced1.diff 2019-11-14 21:45:58.931956527 +0100
@@ -6,24 +6,10 @@
r->entry_count = t->entry_count;
r->delta_depth = t->delta_depth;
- memcpy(r->entries,t->entries,t->entry_count*sizeof(t->entries[0]));
-+ COPY_ARRAY(r->entries, t->entries, t->entry_count);
++ memcpy(r->entries,t->entries,t->entry_count*sizeof(*(t->entries)));
release_tree_content(t);
return r;
}
-diff -u -p a/pretty.c b/pretty.c
a/pretty.c
-+++ b/pretty.c
-@@ -106,8 +106,8 @@ static void setup_commit_formats(void)
- commit_formats_len = ARRAY_SIZE(builtin_formats);
- builtin_formats_len = commit_formats_len;
- ALLOC_GROW(commit_formats, commit_formats_len, commit_formats_alloc);
-- memcpy(commit_formats, builtin_formats,
-- sizeof(*builtin_formats)*ARRAY_SIZE(builtin_formats));
-+ COPY_ARRAY(commit_formats, builtin_formats,
-+ ARRAY_SIZE(builtin_formats));
-
- git_config(git_pretty_formats_config, NULL);
- }
diff -u -p a/packfile.c b/packfile.c
--- a/packfile.c
+++ b/packfile.c
@@ -36,17 +22,6 @@
} else {
ALLOC_GROW(poi_stack, poi_stack_nr+1, poi_stack_alloc);
}
-@@ -1698,8 +1698,8 @@ void *unpack_entry(struct repository *r,
- && delta_stack == small_delta_stack) {
- delta_stack_alloc = alloc_nr(delta_stack_nr);
- ALLOC_ARRAY(delta_stack, delta_stack_alloc);
-- memcpy(delta_stack, small_delta_stack,
-- sizeof(*delta_stack)*delta_stack_nr);
-+ COPY_ARRAY(delta_stack, small_delta_stack,
-+ delta_stack_nr);
- } else {
- ALLOC_GROW(delta_stack, delta_stack_nr+1,
delta_stack_alloc);
- }
diff -u -p a/compat/regex/regexec.c b/compat/regex/regexec.c
--- a/compat/regex/regexec.c
+++ b/compat/regex/regexec.c
How do you think about the differences from this test result?
Regards,
Markus
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
