Re: [PATCH v3 01/30] syntax-check: sc_prohibit_nonreentrant: skip comments

[Victor Toso]

On Thu, Apr 21, 2022 at 02:11:03PM +0200, Peter Krempa wrote:
> On Wed, Apr 20, 2022 at 21:07:50 +0200, Victor Toso wrote:
> > It doesn't need to fail the test if a reentrant function is found in
> > a comment.
> 
> That is true, but your patch doesn't do exactly that.
> 
> If I add the following code:
> 
> @@ -83,6 +83,12 @@ virshCatchDisconnect(virConnectPtr conn,
>  const char *str = "unknown reason";
>  virErrorPtr error;
>  g_autofree char *uri = NULL;
> +long r;
> +
> +r = random(); /* let's play a game */
> +
> +if (r == 4)
> +abort();
> 
> 
> 
> It fails prior to this patch, but succeeds after it.

Yeah.

> > Fixes:
> >  | ...
> >  | prohibit_nonreentrant
> >  | libvirt/ci/helper:33:default=os.getlogin(),  # exempt from 
> > syntax-check
> >  | libvirt/include/libvirt/libvirt-storage.h:223:
> > VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 = 6, /* 7-pass random (Since: v1.0.0) */
> >  | libvirt/include/libvirt/libvirt-storage.h:225:
> > VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 = 7, /* 33-pass random (Since: v1.0.0) 
> > */
> >  | libvirt/include/libvirt/libvirt-storage.h:227:
> > VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random (Since: v1.0.0) */
> 
> Since it's very hard to parse C with regexes I suggest to
> change
> 
>  'pass random' to 'pass random data'
> 
> in the comments above to sidestep the issue.

Sure.

Thanks,
Victor


signature.asc
Description: PGP signature

Re: [PATCH v3 01/30] syntax-check: sc_prohibit_nonreentrant: skip comments

[Peter Krempa]

On Wed, Apr 20, 2022 at 21:07:50 +0200, Victor Toso wrote:
> It doesn't need to fail the test if a reentrant function is found in
> a comment.

That is true, but your patch doesn't do exactly that.

If I add the following code:

@@ -83,6 +83,12 @@ virshCatchDisconnect(virConnectPtr conn,
 const char *str = "unknown reason";
 virErrorPtr error;
 g_autofree char *uri = NULL;
+long r;
+
+r = random(); /* let's play a game */
+
+if (r == 4)
+abort();



It fails prior to this patch, but succeeds after it.

> 
> Fixes:
>  | ...
>  | prohibit_nonreentrant
>  | libvirt/ci/helper:33:default=os.getlogin(),  # exempt from 
> syntax-check
>  | libvirt/include/libvirt/libvirt-storage.h:223:
> VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 = 6, /* 7-pass random (Since: v1.0.0) */
>  | libvirt/include/libvirt/libvirt-storage.h:225:
> VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 = 7, /* 33-pass random (Since: v1.0.0) */
>  | libvirt/include/libvirt/libvirt-storage.h:227:
> VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random (Since: v1.0.0) */

Since it's very hard to parse C with regexes I suggest to change

 'pass random' to 'pass random data'

in the comments above to sidestep the issue.

[PATCH v3 01/30] syntax-check: sc_prohibit_nonreentrant: skip comments

[Victor Toso]

It doesn't need to fail the test if a reentrant function is found in
a comment.

Fixes:
 | ...
 | prohibit_nonreentrant
 | libvirt/ci/helper:33:default=os.getlogin(),  # exempt from 
syntax-check
 | libvirt/include/libvirt/libvirt-storage.h:223:
VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7 = 6, /* 7-pass random (Since: v1.0.0) */
 | libvirt/include/libvirt/libvirt-storage.h:225:
VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33 = 7, /* 33-pass random (Since: v1.0.0) */
 | libvirt/include/libvirt/libvirt-storage.h:227:
VIR_STORAGE_VOL_WIPE_ALG_RANDOM = 8, /* 1-pass random (Since: v1.0.0) */

Signed-off-by: Victor Toso 
---
 build-aux/syntax-check.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk
index a6e3ada300..eb403f3d3f 100644
--- a/build-aux/syntax-check.mk
+++ b/build-aux/syntax-check.mk
@@ -300,7 +300,7 @@ sc_prohibit_PATH_MAX:
 include $(top_srcdir)/build-aux/Makefile.nonreentrant
 sc_prohibit_nonreentrant:
@prohibit="\\<(${NON_REENTRANT_RE}) *\\(" \
-   exclude='exempt from syntax-check' \
+   exclude='exempt from syntax-check|\/\*.*\*\/' \
halt="use re-entrant functions (usually ending with _r)" \
  $(_sc_search_regexp)
 
-- 
2.35.1