Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Remove use of global vars for current id and param name #51

Closed
wants to merge 3 commits into from

Conversation

BenMatase
Copy link

Fixes #44

This avoids a race when trying to populate multiple SyslogMessages concurrently.

I've tested this out in a scenario with telegraf where we were seeing this issue frequently and now we don't.

@BenMatase
Copy link
Author

@leodido can't add you to reviewers

@leodido leodido self-requested a review November 29, 2023 19:55
Copy link
Collaborator

@leodido leodido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should apply those changes in the Ragel file (ie., builder.go.rl if I don't remember wrong). Then compile it in Go with the existing makefile.

@BenMatase
Copy link
Author

@leodido it changed a lot in the .go file. Is this error output the reason?

[bmatase@bmatase-mbp go-syslog (fix-global-var-race)]$ make build
ragel -I common -Z -G2 -e -o rfc5424/builder.go rfc5424/builder.go.rl
rfc5424/builder.go.rl:108:37: warning: applying plus operator to a machine that accepts zero length word
sed: -I or -i may not be used with stdin
make: *** [rfc5424/builder.go] Error 1

@BenMatase
Copy link
Author

I see the problem. Working on fixing it

@BenMatase
Copy link
Author

@leodido had some trouble with the -i aka inplace flags on my mac but I think it is in good shape now

@BenMatase
Copy link
Author

@leodido I believe I've addressed the changes you requested. Let me know if there is anything else you spot

@leodido
Copy link
Collaborator

leodido commented May 19, 2024

I don't have anymore write/maintain access here. So, I can't help/maintain it. It's unfortunate but it is what it is... Feel free to redirect this PR to https://github.com/leodido/go-syslog. I plan to keep evolving this project there, on my GitHub. Thank you

@tkyocum tkyocum closed this May 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data race on currentid
3 participants