[Tarantool-patches] [PATCH v2 2/2] lua: abort trace recording on fiber yield
Igor Munkin
imun at tarantool.org
Wed Sep 30 09:27:25 MSK 2020
Vlad,
Thanks for your review!
On 30.09.20, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> Functionally all looks good. See one comment related not to Lua.
>
> > diff --git a/src/lua/utils.c b/src/lua/utils.c
> > index 8e98f7607..6e38f5734 100644
> > --- a/src/lua/utils.c
> > +++ b/src/lua/utils.c
> > @@ -1309,10 +1310,61 @@ tarantool_lua_utils_init(struct lua_State *L)
> > return 0;
> > }
> >
> > +/*
> > + * XXX: There is already defined <panic> macro in say.h header
> > + * (included in diag.h). As a result the call below is misexpanded
> > + * and compilation fails with the corresponding error. To avoid
> > + * this error the macro is temporary renamed and restored later.
> > + * Compilation now fails for the following cases:
> > + * * temporary name <gh1700_panic> is used in scope of this
> > + * translation unit
> > + * * <panic> is not define, so this hack can be freely dropped
> > + */
> > +#if defined(panic) && !defined(gh1700_panic)
> > +# define gh1700_panic panic
> > +# undef panic
> > +#else
> > +# error "Can't redefine <panic> macro"
> > +#endif
>
> Unfortunately, it won't work. Macros are not variables. You
> can's assign them like that. I wish we could.
>
> What you did here is you eliminated 'panic' macro, and added a
> new macro 'gh1700_panic' whose value is "panic". Not what was
> in the old 'panic', but exactly "panic". So the old value of 'panic'
> is lost here.
Hell, you're right. They can be recursively expanded, but if panic macro
is undefined then it is just a bareword (as I originally wanted).
>
<snipped>
>
> Also when you try to restore the old 'panic', it also won't work
> again. As an alternative you can just do '#undef panic', since your
> code is in the end of the file anyway, it won't break shit. Or make
> a separate patch, which would turn panic() into a function.
I simply undefined <panic> macro (the diff is below) and filed an
issue[1] to rewrite this part.
================================================================================
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 6e38f5734..bb2287162 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -1314,18 +1314,10 @@ tarantool_lua_utils_init(struct lua_State *L)
* XXX: There is already defined <panic> macro in say.h header
* (included in diag.h). As a result the call below is misexpanded
* and compilation fails with the corresponding error. To avoid
- * this error the macro is temporary renamed and restored later.
- * Compilation now fails for the following cases:
- * * temporary name <gh1700_panic> is used in scope of this
- * translation unit
- * * <panic> is not define, so this hack can be freely dropped
+ * this error the macro is undefined since it's not used anymore
+ * in scope of this translation unit.
*/
-#if defined(panic) && !defined(gh1700_panic)
-# define gh1700_panic panic
-# undef panic
-#else
-# error "Can't redefine <panic> macro"
-#endif
+#undef panic
/**
* This routine encloses the checks and actions to be done when
@@ -1364,7 +1356,3 @@ void cord_on_yield(void)
*/
lj_trace_abort(g);
}
-
-/* Restore <panic> macro back */
-#define panic gh1700_panic
-#undef gh1700_panic
================================================================================
[1]: https://github.com/tarantool/tarantool/issues/5365
--
Best regards,
IM
More information about the Tarantool-patches
mailing list