* [Cocci] Itterating matches @ 2020-11-28 5:41 Ira Weiny 2020-11-28 7:55 ` Julia Lawall [not found] ` <13b5bbf1-4dda-9dc2-46cd-a98e71537769@web.de> 0 siblings, 2 replies; 7+ messages in thread From: Ira Weiny @ 2020-11-28 5:41 UTC (permalink / raw) To: cocci I have been looking for a way to help automate my kmap/mem*/kunmap transformation.[1] I'm very new to coccinelle and I'm having some trouble. I have tried to build up a script[2][3] to do this while also learning coccinelle but I'm not getting a couple of points. 1) How do I get matches to iterate? 1a) What is the difference between '<+... ...+>' and '<... ...>'? 2) Why can't I get the type declaration (type VP;) matched correctly. It works some times but not always. It also matches a lot of random declarations? 2a) and for some reason in the more advanced script[3] it completely breaks. Which is very confusing because fundamentally it does not seem to be any different from [2]? Any advice is appreciated. Thanks, Ira [1] https://lore.kernel.org/lkml/20201124141244.GE17322@twin.jikos.cz/ [2] <script> 1 @ rule0 @ 2 type VP; 3 identifier ptr; 4 identifier page; 5 @@ 6 7 -VP ptr; 8 <... 9 ( 10 -ptr = kmap(page); 11 | 12 -ptr = kmap_atomic(page); 13 ) 14 <+... 15 ( 16 -memset(...); 17 +memIra(); 18 | 19 -memcpy(...); 20 +memIra(); 21 ) 22 ...+> 23 ( 24 -kunmap(page); 25 | 26 -kunmap_atomic(ptr); 27 ) 28 ...> </script> [3] <script> 1 @ rule0 @ 2 type VP; 3 identifier page; 4 identifier ptr; 5 expression V; 6 expression L; 7 expression T; 8 expression F; 9 expression B; 10 expression Off; 11 @@ 12 13 -VP ptr; 14 <... 15 ( 16 -ptr = kmap(page); 17 | 18 -ptr = kmap_atomic(page); 19 ) 20 <... 21 ( 22 -memset(ptr, 0, L); 23 +zero_user(page, 0, L); 24 | 25 -memset(ptr + Off, 0, L); 26 +zero_user(page, Off, L); 27 | 28 -memset(ptr, V, L); 29 +memset_page(page, V, 0, L); 30 | 31 -memset(ptr + Off, V, L); 32 +memset_page(page, V, Off, L); 33 | 34 -memcpy(ptr + Off, F, B); 35 +memcpy_to_page(page, Off, F, B); 36 | 37 -memcpy(ptr, F, B); 38 +memcpy_to_page(page, 0, F, B); 39 | 40 -memcpy(T, ptr + Off, B); 41 +memcpy_from_page(T, page, Off, B); 42 | 43 -memcpy(T, ptr, B); 44 +memcpy_from_page(T, page, 0, B); 45 ) 46 ...> 47 ( 48 -kunmap(page); 49 | 50 -kunmap_atomic(ptr); 51 ) 52 ...> </script> _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Cocci] Itterating matches 2020-11-28 5:41 [Cocci] Itterating matches Ira Weiny @ 2020-11-28 7:55 ` Julia Lawall 2020-11-30 19:19 ` Ira Weiny [not found] ` <13b5bbf1-4dda-9dc2-46cd-a98e71537769@web.de> 1 sibling, 1 reply; 7+ messages in thread From: Julia Lawall @ 2020-11-28 7:55 UTC (permalink / raw) To: Ira Weiny; +Cc: cocci On Fri, 27 Nov 2020, Ira Weiny wrote: > I have been looking for a way to help automate my kmap/mem*/kunmap > transformation.[1] I'm very new to coccinelle and I'm having some trouble. > > I have tried to build up a script[2][3] to do this while also learning coccinelle > but I'm not getting a couple of points. > > 1) How do I get matches to iterate? What do you mean by iterate? > 1a) What is the difference between '<+... ...+>' and '<... ...>'? <+... P ...+> requires P to appear one or more times <... P ...> allows P to occur 0 or more times > 2) Why can't I get the type declaration (type VP;) matched correctly. It works > some times but not always. It also matches a lot of random declarations? > 2a) and for some reason in the more advanced script[3] it completely > breaks. Which is very confusing because fundamentally it does not > seem to be any different from [2]? I haven't tried running the scripts to see why the declaration breaks. but I think that the declaration is not what you want. ptr could be used in some other case. It would be better to have one rule that makes the rest of the changes in the code, and the another rule that removes the ptr declaration when there is no remaining use of the variable: @r1@ identifier ptr; ... @@ remove kmaps etc @@ identifier r1.ptr; type T; @@ - T ptr; ... when != ptr This will allow you to get rid of lines 8 and 28 in [2]. julia > Any advice is appreciated. > > Thanks, > Ira > > [1] https://lore.kernel.org/lkml/20201124141244.GE17322@twin.jikos.cz/ > > [2] > <script> > 1 @ rule0 @ > 2 type VP; > 3 identifier ptr; > 4 identifier page; > 5 @@ > 6 > 7 -VP ptr; > 8 <... > 9 ( > 10 -ptr = kmap(page); > 11 | > 12 -ptr = kmap_atomic(page); > 13 ) > 14 <+... > 15 ( > 16 -memset(...); > 17 +memIra(); > 18 | > 19 -memcpy(...); > 20 +memIra(); > 21 ) > 22 ...+> > 23 ( > 24 -kunmap(page); > 25 | > 26 -kunmap_atomic(ptr); > 27 ) > 28 ...> > </script> > > [3] > <script> > 1 @ rule0 @ > 2 type VP; > 3 identifier page; > 4 identifier ptr; > 5 expression V; > 6 expression L; > 7 expression T; > 8 expression F; > 9 expression B; > 10 expression Off; > 11 @@ > 12 > 13 -VP ptr; > 14 <... > 15 ( > 16 -ptr = kmap(page); > 17 | > 18 -ptr = kmap_atomic(page); > 19 ) > 20 <... > 21 ( > 22 -memset(ptr, 0, L); > 23 +zero_user(page, 0, L); > 24 | > 25 -memset(ptr + Off, 0, L); > 26 +zero_user(page, Off, L); > 27 | > 28 -memset(ptr, V, L); > 29 +memset_page(page, V, 0, L); > 30 | > 31 -memset(ptr + Off, V, L); > 32 +memset_page(page, V, Off, L); > 33 | > 34 -memcpy(ptr + Off, F, B); > 35 +memcpy_to_page(page, Off, F, B); > 36 | > 37 -memcpy(ptr, F, B); > 38 +memcpy_to_page(page, 0, F, B); > 39 | > 40 -memcpy(T, ptr + Off, B); > 41 +memcpy_from_page(T, page, Off, B); > 42 | > 43 -memcpy(T, ptr, B); > 44 +memcpy_from_page(T, page, 0, B); > 45 ) > 46 ...> > 47 ( > 48 -kunmap(page); > 49 | > 50 -kunmap_atomic(ptr); > 51 ) > 52 ...> > </script> > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Cocci] Itterating matches 2020-11-28 7:55 ` Julia Lawall @ 2020-11-30 19:19 ` Ira Weiny 2020-11-30 19:59 ` Julia Lawall 0 siblings, 1 reply; 7+ messages in thread From: Ira Weiny @ 2020-11-30 19:19 UTC (permalink / raw) To: Julia Lawall; +Cc: cocci On Sat, Nov 28, 2020 at 08:55:16AM +0100, Julia Lawall wrote: > > > On Fri, 27 Nov 2020, Ira Weiny wrote: > > > I have been looking for a way to help automate my kmap/mem*/kunmap > > transformation.[1] I'm very new to coccinelle and I'm having some trouble. > > > > I have tried to build up a script[2][3] to do this while also learning coccinelle > > but I'm not getting a couple of points. > > > > 1) How do I get matches to iterate? > > What do you mean by iterate? > > > 1a) What is the difference between '<+... ...+>' and '<... ...>'? > > <+... P ...+> requires P to appear one or more times > <... P ...> allows P to occur 0 or more times Great that is what I thought but I was getting confused. See the examples below for details. > > > 2) Why can't I get the type declaration (type VP;) matched correctly. It works > > some times but not always. It also matches a lot of random declarations? > > 2a) and for some reason in the more advanced script[3] it completely > > breaks. Which is very confusing because fundamentally it does not > > seem to be any different from [2]? > > I haven't tried running the scripts to see why the declaration breaks. > but I think that the declaration is not what you want. ptr could be used > in some other case. It would be better to have one rule that makes the > rest of the changes in the code, and the another rule that removes the ptr > declaration when there is no remaining use of the variable: > > @r1@ > identifier ptr; > ... > @@ > > remove kmaps etc > > @@ > identifier r1.ptr; > type T; > @@ > > - T ptr; > ... when != ptr > > This will allow you to get rid of lines 8 and 28 in [2]. Ok this helps a lot. Thank you! That said it does not seem to work. I have a better example now. https://elixir.bootlin.com/linux/v5.10-rc6/source/fs/btrfs/compression.c fs/btrfs/compression.c: 571 if (page->index == end_index) { 572 char *userpage; 573 size_t zero_offset = offset_in_page(isize); 574 575 if (zero_offset) { 576 int zeros; 577 zeros = PAGE_SIZE - zero_offset; 578 userpage = kmap_atomic(page); 579 memset(userpage + zero_offset, 0, zeros); 580 flush_dcache_page(page); 581 kunmap_atomic(userpage); 582 } 583 } <script> @ r1 @ identifier page; identifier ptr; expression L; expression Off; @@ -ptr = kmap_atomic(page); ... -memset(ptr + Off, 0, L); +zero_user(page, Off, L); ... -kunmap_atomic(ptr); @@ identifier r1.ptr; type VP; @@ -VP ptr; ... when != ptr; </script> Results in: --- fs/btrfs/compression.c +++ /tmp/cocci-output-1442863-cb5dd7-compression.c @@ -575,10 +575,8 @@ static noinline int add_ra_bio_pages(str if (zero_offset) { int zeros; zeros = PAGE_SIZE - zero_offset; - userpage = kmap_atomic(page); - memset(userpage + zero_offset, 0, zeros); + zero_user(page, zero_offset, zeros); flush_dcache_page(page); - kunmap_atomic(userpage); } } Question: Is there something about userpage being declared in the outer block which is causing the second rule to fail? Back to the 'iteration issue'. I've realized that I understood perfectly what the iteration dot notations meant... My bug was elsewhere. For others who may be following along at home, I'll walk through this because it brings up another issue with the removal of the pointer declaration which I don't understand. That same file (fs/btrfs/compression.c) has a second instance I'm trying to match; except with memcpy(). 1242 char *kaddr; ... 1267 /* copy bytes from the working buffer into the pages */ 1268 while (working_bytes > 0) { 1269 bytes = min_t(unsigned long, bvec.bv_len, 1270 PAGE_SIZE - (buf_offset % PAGE_SIZE)); 1271 bytes = min(bytes, working_bytes); 1272 1273 kaddr = kmap_atomic(bvec.bv_page); 1274 memcpy(kaddr + bvec.bv_offset, buf + buf_offset, bytes); 1275 kunmap_atomic(kaddr); ... Here I added <... ...> (and even tried <+... ...+>) around the k[un]map_atomic() in my script to try and catch the second instance along with the memcpy() as an alternative match; thusly. <script> @ r1 @ identifier page; identifier ptr; expression L; expression F; expression B; expression Off; @@ <... -ptr = kmap_atomic(page); ... ( -memset(ptr + Off, 0, L); +zero_user(page, Off, L); | -memcpy(ptr + Off, F, B); +memcpy_to_page(page, Off, F, B); ) ... -kunmap_atomic(ptr); ...> @@ identifier r1.ptr; type VP; @@ -VP ptr; ... when != ptr; </script> Which results in: --- fs/btrfs/compression.c +++ /tmp/cocci-output-1443266-8381d0-compression.c @@ -575,10 +575,8 @@ static noinline int add_ra_bio_pages(str if (zero_offset) { int zeros; zeros = PAGE_SIZE - zero_offset; - userpage = kmap_atomic(page); - memset(userpage + zero_offset, 0, zeros); + zero_user(page, zero_offset, zeros); flush_dcache_page(page); - kunmap_atomic(userpage); } } :-/ Finally, after thinking on this more I realized this was _not_ because the _iteration_ was wrong. Rather the second kmap_atomic uses 'bvec.bv_page' which matches as an expression not an identifier. The correct script is therefore: <script> 10:59:30 > cat kmap-test.cocci @ r1 @ expression page; <============ 'expression' identifier ptr; expression L; expression F; expression B; expression Off; @@ -ptr = kmap_atomic(page); ... ( -memset(ptr + Off, 0, L); +zero_user(page, Off, L); | -memcpy(ptr + Off, F, B); +memcpy_to_page(page, Off, F, B); ) ... -kunmap_atomic(ptr); @@ identifier r1.ptr; type VP; @@ -VP ptr; ... when != ptr; </script> which results in... --- fs/btrfs/compression.c +++ /tmp/cocci-output-1443375-182695-compression.c @@ -145,7 +145,6 @@ static int check_compressed_csum(struct const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy); struct page *page; unsigned long i; - char *kaddr; u8 csum[BTRFS_CSUM_SIZE]; struct compressed_bio *cb = bio->bi_private; u8 *cb_sum = cb->sums; @@ -575,10 +574,8 @@ static noinline int add_ra_bio_pages(str if (zero_offset) { int zeros; zeros = PAGE_SIZE - zero_offset; - userpage = kmap_atomic(page); - memset(userpage + zero_offset, 0, zeros); + zero_user(page, zero_offset, zeros); flush_dcache_page(page); - kunmap_atomic(userpage); } } @@ -1239,7 +1236,6 @@ int btrfs_decompress_buf2page(const char unsigned long prev_start_byte; unsigned long working_bytes = total_out - buf_start; unsigned long bytes; - char *kaddr; struct bio_vec bvec = bio_iter_iovec(bio, bio->bi_iter); /* @@ -1270,9 +1266,8 @@ int btrfs_decompress_buf2page(const char PAGE_SIZE - (buf_offset % PAGE_SIZE)); bytes = min(bytes, working_bytes); - kaddr = kmap_atomic(bvec.bv_page); - memcpy(kaddr + bvec.bv_offset, buf + buf_offset, bytes); - kunmap_atomic(kaddr); + memcpy_to_page(bvec.bv_page, bvec.bv_offset, buf + buf_offset, + bytes); flush_dcache_page(bvec.bv_page); buf_offset += bytes; Yay!!! The 'iteration' is not even needed... :-D So sorry for bothering you on that point. However, I still have an issue with the second rule you specified. It appears to be too aggressive with removing kaddr; removing it from an entirely unrelated function. Why? Is there a way to limit the scope of the second rule to blocks and parent blocks? And the final questoin is: Why is kaddr correctly removed from btrfs_decompress_buf2page() while the previous match 'userpage' is not? It seems like exactly the same pattern... So obviously don't fully understand. Thanks so much for your help though! :-D Ira _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Cocci] Itterating matches 2020-11-30 19:19 ` Ira Weiny @ 2020-11-30 19:59 ` Julia Lawall 2020-11-30 21:06 ` Ira Weiny 0 siblings, 1 reply; 7+ messages in thread From: Julia Lawall @ 2020-11-30 19:59 UTC (permalink / raw) To: Ira Weiny; +Cc: cocci > That said it does not seem to work. I have a better example now. > > https://elixir.bootlin.com/linux/v5.10-rc6/source/fs/btrfs/compression.c > > fs/btrfs/compression.c: > > 571 if (page->index == end_index) { > 572 char *userpage; > 573 size_t zero_offset = offset_in_page(isize); > 574 > 575 if (zero_offset) { > 576 int zeros; > 577 zeros = PAGE_SIZE - zero_offset; > 578 userpage = kmap_atomic(page); > 579 memset(userpage + zero_offset, 0, zeros); > 580 flush_dcache_page(page); > 581 kunmap_atomic(userpage); > 582 } > 583 } > > <script> > @ r1 @ > identifier page; > identifier ptr; > expression L; > expression Off; > @@ > > -ptr = kmap_atomic(page); > ... > -memset(ptr + Off, 0, L); > +zero_user(page, Off, L); > ... > -kunmap_atomic(ptr); > > > @@ > identifier r1.ptr; > type VP; > @@ > > -VP ptr; > ... when != ptr; > </script> > > Results in: > > --- fs/btrfs/compression.c > +++ /tmp/cocci-output-1442863-cb5dd7-compression.c > @@ -575,10 +575,8 @@ static noinline int add_ra_bio_pages(str > if (zero_offset) { > int zeros; > zeros = PAGE_SIZE - zero_offset; > - userpage = kmap_atomic(page); > - memset(userpage + zero_offset, 0, zeros); > + zero_user(page, zero_offset, zeros); > flush_dcache_page(page); > - kunmap_atomic(userpage); > } > } > > > Question: Is there something about userpage being declared in the outer block > which is causing the second rule to fail? The problem is the while loop. The ... when != ptr goes to the end of the function. It thus goes around the loop and reaches the use of userpage again. You can do: type VP, VP1; - VP ptr; ... when != ptr; ? VP1 ptr; Then it will check that there is no use of ptr until reaching a new declaration of ptr, if any (the ? ). > > > Back to the 'iteration issue'. I've realized that I understood perfectly what > the iteration dot notations meant... My bug was elsewhere. > > For others who may be following along at home, I'll walk through this because > it brings up another issue with the removal of the pointer declaration which > I don't understand. > > That same file (fs/btrfs/compression.c) has a second instance I'm trying to > match; except with memcpy(). > > 1242 char *kaddr; > ... > 1267 /* copy bytes from the working buffer into the pages */ > 1268 while (working_bytes > 0) { > 1269 bytes = min_t(unsigned long, bvec.bv_len, > 1270 PAGE_SIZE - (buf_offset % PAGE_SIZE)); > 1271 bytes = min(bytes, working_bytes); > 1272 > 1273 kaddr = kmap_atomic(bvec.bv_page); > 1274 memcpy(kaddr + bvec.bv_offset, buf + buf_offset, bytes); > 1275 kunmap_atomic(kaddr); > ... > > > Here I added <... ...> (and even tried <+... ...+>) around the > k[un]map_atomic() in my script to try and catch the second instance along with > the memcpy() as an alternative match; thusly. > > <script> > @ r1 @ > identifier page; > identifier ptr; > expression L; > expression F; > expression B; > expression Off; > @@ > > <... > -ptr = kmap_atomic(page); > ... > ( > -memset(ptr + Off, 0, L); > +zero_user(page, Off, L); > | > -memcpy(ptr + Off, F, B); > +memcpy_to_page(page, Off, F, B); > ) > ... > -kunmap_atomic(ptr); > ...> I don't see the need for the outer <... ...>. It looks like the pattern inside the <... ...> should already match lines 1273-1275 in the code. > > @@ > identifier r1.ptr; > type VP; > @@ > > -VP ptr; > ... when != ptr; > </script> > > Which results in: > > --- fs/btrfs/compression.c > +++ /tmp/cocci-output-1443266-8381d0-compression.c > @@ -575,10 +575,8 @@ static noinline int add_ra_bio_pages(str > if (zero_offset) { > int zeros; > zeros = PAGE_SIZE - zero_offset; > - userpage = kmap_atomic(page); > - memset(userpage + zero_offset, 0, zeros); > + zero_user(page, zero_offset, zeros); > flush_dcache_page(page); > - kunmap_atomic(userpage); > } > } > > :-/ > > Finally, after thinking on this more I realized this was _not_ because the > _iteration_ was wrong. Rather the second kmap_atomic uses 'bvec.bv_page' which > matches as an expression not an identifier. The correct script is therefore: > > <script> > 10:59:30 > cat kmap-test.cocci > @ r1 @ > expression page; <============ 'expression' > identifier ptr; > expression L; > expression F; > expression B; > expression Off; > @@ > > -ptr = kmap_atomic(page); > ... > ( > -memset(ptr + Off, 0, L); > +zero_user(page, Off, L); > | > -memcpy(ptr + Off, F, B); > +memcpy_to_page(page, Off, F, B); > ) > ... > -kunmap_atomic(ptr); > > > @@ > identifier r1.ptr; > type VP; > @@ > > -VP ptr; > ... when != ptr; > </script> > > which results in... > > --- fs/btrfs/compression.c > +++ /tmp/cocci-output-1443375-182695-compression.c > @@ -145,7 +145,6 @@ static int check_compressed_csum(struct > const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy); > struct page *page; > unsigned long i; > - char *kaddr; > u8 csum[BTRFS_CSUM_SIZE]; > struct compressed_bio *cb = bio->bi_private; > u8 *cb_sum = cb->sums; > @@ -575,10 +574,8 @@ static noinline int add_ra_bio_pages(str > if (zero_offset) { > int zeros; > zeros = PAGE_SIZE - zero_offset; > - userpage = kmap_atomic(page); > - memset(userpage + zero_offset, 0, zeros); > + zero_user(page, zero_offset, zeros); > flush_dcache_page(page); > - kunmap_atomic(userpage); > } > } > > @@ -1239,7 +1236,6 @@ int btrfs_decompress_buf2page(const char > unsigned long prev_start_byte; > unsigned long working_bytes = total_out - buf_start; > unsigned long bytes; > - char *kaddr; > struct bio_vec bvec = bio_iter_iovec(bio, bio->bi_iter); > > /* > @@ -1270,9 +1266,8 @@ int btrfs_decompress_buf2page(const char > PAGE_SIZE - (buf_offset % PAGE_SIZE)); > bytes = min(bytes, working_bytes); > > - kaddr = kmap_atomic(bvec.bv_page); > - memcpy(kaddr + bvec.bv_offset, buf + buf_offset, bytes); > - kunmap_atomic(kaddr); > + memcpy_to_page(bvec.bv_page, bvec.bv_offset, buf + buf_offset, > + bytes); > flush_dcache_page(bvec.bv_page); > > buf_offset += bytes; > > > Yay!!! The 'iteration' is not even needed... :-D So sorry for bothering you > on that point. > > However, I still have an issue with the second rule you specified. It appears > to be too aggressive with removing kaddr; removing it from an entirely > unrelated function. Why? Is there a way to limit the scope of the second rule > to blocks and parent blocks? Indeed the rule does nothing to check that the function is the same. I kind of assumed that good code would not contain unused variables, and if the code does contain unused variables you would be happy to be rid of them. However, it is possible with some scripting hacks to find the name of the function that one rule matches and then use that information to filter the matches of another rule. But again, I'm not sure it is worth the trouble. julia > > And the final questoin is: Why is kaddr correctly removed from > btrfs_decompress_buf2page() while the previous match 'userpage' is not? It > seems like exactly the same pattern... > > So obviously don't fully understand. > > Thanks so much for your help though! :-D > Ira > > _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Cocci] Itterating matches 2020-11-30 19:59 ` Julia Lawall @ 2020-11-30 21:06 ` Ira Weiny 2020-12-01 10:06 ` Julia Lawall 0 siblings, 1 reply; 7+ messages in thread From: Ira Weiny @ 2020-11-30 21:06 UTC (permalink / raw) To: Julia Lawall; +Cc: cocci On Mon, Nov 30, 2020 at 08:59:42PM +0100, Julia Lawall wrote: [snip] > > > > > > Question: Is there something about userpage being declared in the outer block > > which is causing the second rule to fail? > > The problem is the while loop. The ... when != ptr goes to the end of the > function. It thus goes around the loop and reaches the use of userpage > again. You can do: > > type VP, VP1; > > - VP ptr; > ... when != ptr; > ? VP1 ptr; > > Then it will check that there is no use of ptr until reaching a new > declaration of ptr, if any (the ? ). Thanks! [snip] > > > > > > Yay!!! The 'iteration' is not even needed... :-D So sorry for bothering you > > on that point. > > > > However, I still have an issue with the second rule you specified. It appears > > to be too aggressive with removing kaddr; removing it from an entirely > > unrelated function. Why? Is there a way to limit the scope of the second rule > > to blocks and parent blocks? > > Indeed the rule does nothing to check that the function is the same. I > kind of assumed that good code would not contain unused variables, and if > the code does contain unused variables you would be happy to be rid of > them. But the kaddr in check_compressed_csum() is used? 148 char *kaddr; 149 u8 csum[BTRFS_CSUM_SIZE]; 150 struct compressed_bio *cb = bio->bi_private; 151 u8 *cb_sum = cb->sums; 152 153 if (inode->flags & BTRFS_INODE_NODATASUM) 154 return 0; 155 156 shash->tfm = fs_info->csum_shash; 157 158 for (i = 0; i < cb->nr_pages; i++) { 159 page = cb->compressed_pages[i]; 160 161 kaddr = kmap_atomic(page); 162 crypto_shash_digest(shash, kaddr, PAGE_SIZE, csum); 163 kunmap_atomic(kaddr); It correctly leaves this kmap_atomic alone for me. > However, it is possible with some scripting hacks to find the name > of the function that one rule matches and then use that information to > filter the matches of another rule. But again, I'm not sure it is worth > the trouble. Nope... I can deal with it. :-D This is so much better than what I had. Thanks again for your help, Ira _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Cocci] Itterating matches 2020-11-30 21:06 ` Ira Weiny @ 2020-12-01 10:06 ` Julia Lawall 0 siblings, 0 replies; 7+ messages in thread From: Julia Lawall @ 2020-12-01 10:06 UTC (permalink / raw) To: Ira Weiny; +Cc: cocci > But the kaddr in check_compressed_csum() is used? > > 148 char *kaddr; > 149 u8 csum[BTRFS_CSUM_SIZE]; > 150 struct compressed_bio *cb = bio->bi_private; > 151 u8 *cb_sum = cb->sums; > 152 > 153 if (inode->flags & BTRFS_INODE_NODATASUM) > 154 return 0; > 155 > 156 shash->tfm = fs_info->csum_shash; > 157 > 158 for (i = 0; i < cb->nr_pages; i++) { > 159 page = cb->compressed_pages[i]; > 160 > 161 kaddr = kmap_atomic(page); > 162 crypto_shash_digest(shash, kaddr, PAGE_SIZE, csum); > 163 kunmap_atomic(kaddr); > > It correctly leaves this kmap_atomic alone for me. OK, that's strange. I will take a look. julia _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <13b5bbf1-4dda-9dc2-46cd-a98e71537769@web.de>]
* Re: [Cocci] Automating a kmap/mem*/kunmap transformation [not found] ` <13b5bbf1-4dda-9dc2-46cd-a98e71537769@web.de> @ 2020-11-30 22:48 ` Ira Weiny 0 siblings, 0 replies; 7+ messages in thread From: Ira Weiny @ 2020-11-30 22:48 UTC (permalink / raw) To: Markus Elfring; +Cc: cocci On Sun, Nov 29, 2020 at 11:17:14AM +0100, Markus Elfring wrote: > > 1) How do I get matches to iterate? > > How do you think about possibilities for “iteration”? > > > > 1a) What is the difference between '<+... ...+>' and '<... ...>'? > > I suggest to take another look at the section “Dot variants” in the available > software documentation. > https://coccinelle.gitlabpages.inria.fr/website/docs/main_grammar004.html#sec7 > > Would you like to distinguish if you are looking for source code parts > which can be optional? Yes, not every file will have the pattern I'm looking for and as I explained in my other thread the second instance of that pattern was not being matched and I could not understand why. In that case either '+' or no '+' should (and as I know now does) work. But I thought there was some secret to them I was missing... Sorry... > > > > 2) Why can't I get the type declaration (type VP;) matched correctly. It works > > some times but not always. It also matches a lot of random declarations? > > I imagine that it will help to point a few source code places out > where you find the shown transformation approach improvable. > > > > 2a) and for some reason in the more advanced script[3] it completely breaks. > > How does such an undesirable failure look like? > > I wonder a bit more about the detail why you would like to enclose > the SmPL code by the construct “<... … ...>” here. Because of the second instance of my pattern not matching I thought I needed to specify that the pattern could appear 0 or more times. But I now realize that the pattern specification was wrong. > I find that the change specification “-VP ptr;” would mean then that > all variable declarations should be deleted for selected source files. > > > Would you become interested in any more code alternatives for scripts > in the semantic patch language? > > Examples: > A) > ( > -ptr = kmap(page); > | > -ptr = kmap_atomic(page); > ) > > > This SmPL disjunction for a deletion approach can be transformed > like the following. > > -ptr = \( kmap \| kmap_atomic \) (page); Thanks yes that helps. > > > B) > ( > -memset(...); > +memIra(); > | > -memcpy(...); > +memIra(); > ) > > > This SmPL disjunction for a replacement approach can eventually be transformed > like the following. > > -\( memset \| memcpy \) (...) > +memIra() > ; This makes sense but it is not my final goal and the final goal requires more manipulation of the parameters to memset/memcpy. But I am curious to learn more so I'll keep these alternatives in mind. Thanks! Ira _______________________________________________ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-01 10:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-28 5:41 [Cocci] Itterating matches Ira Weiny 2020-11-28 7:55 ` Julia Lawall 2020-11-30 19:19 ` Ira Weiny 2020-11-30 19:59 ` Julia Lawall 2020-11-30 21:06 ` Ira Weiny 2020-12-01 10:06 ` Julia Lawall [not found] ` <13b5bbf1-4dda-9dc2-46cd-a98e71537769@web.de> 2020-11-30 22:48 ` [Cocci] Automating a kmap/mem*/kunmap transformation Ira Weiny
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).