Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sieve_eval_bc:B_REDIRECT make the function reentrant #5020

Merged

Conversation

dilyanpalauzov
Copy link
Contributor

make by_value non-static, so that libcyrus_sieve can be used to interpret bytecode in multi-threaded environment.

I use sieve_eval_bc() in a multi-threaded application.

make by_value non-static, so that libcyrus_sieve can be
used to interpret bytecode in multi-threaded environment.
@ksmurchison
Copy link
Contributor

This looks good, but instead of this:

                   by-mode  = "N" / "R"           ; "Notify" or "Return"
                   by-trace = "T"                 ; "Trace"
                 */
-                static char by_value[14];
-                snprintf(by_value, sizeof(by_value), "%+ld;%c%s",
+                deliverby = xmalloc(14);
+               snprintf(deliverby, 14, "%+ld;%c%s",
                          sec, toupper(bymode[0]), cmd.u.r.bytrace ? "T" : "");
-                deliverby = by_value;
             }

            if (i->edited_headers) i->getheadersection(m, &headers);

I think I'd prefer this:


                   by-mode  = "N" / "R"           ; "Notify" or "Return"
                   by-trace = "T"                 ; "Trace"
                 */
-                static char by_value[14];
-                snprintf(by_value, sizeof(by_value), "%+ld;%c%s",
-                         sec, toupper(bymode[0]), cmd.u.r.bytrace ? "T" : "");
-                deliverby = by_value;
+                struct buf by_value = BUF_INITIALIZER;
+                buf_printf(&by_value, "%+ld;%c%s",
+                           sec, toupper(bymode[0]), cmd.u.r.bytrace ? "T" : "");
+                deliverby = buf_release(&by_value);
             }
 
             if (i->edited_headers) i->getheadersection(m, &headers);

@dilyanpalauzov
Copy link
Contributor Author

This much more code without benefit.

@rsto
Copy link
Member

rsto commented Sep 5, 2024

I agree with @ksmurchison here. buf_printf is worthwhile here because it's cost is negligible over snprintf but we don't have to deal with hard-coded string lengths.

@ksmurchison
Copy link
Contributor

@dilyanpalauzov Do you want to make the change to use struct buf, or do you want me to create a new PR based on yours?

@dilyanpalauzov
Copy link
Contributor Author

I prefer if you create the change.

@ksmurchison ksmurchison merged commit 0205bb1 into cyrusimap:master Sep 19, 2024
1 check passed
@dilyanpalauzov dilyanpalauzov deleted the sieve_eval_bc_make_reentrant branch September 19, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants