-
Notifications
You must be signed in to change notification settings - Fork 39
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
nxcomp/src/Loop.cpp: Support NXTransDialog caption / title being pref… #1079
base: 3.6.x
Are you sure you want to change the base?
nxcomp/src/Loop.cpp: Support NXTransDialog caption / title being pref… #1079
Conversation
c430f1e
to
eb8c54a
Compare
eb8c54a
to
af91cad
Compare
else | ||
{ | ||
// Use what gets provided via NX_DIALOG_CAPTIONPREFIX (used in X2Go) | ||
strncpy(caption, caption_prefix, DEFAULT_STRING_LENGTH-1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this strncpy is different from the one above regarding the number of characters to be copied. While both are technically correct they should at least be unified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you suggest a clean solution? (You are the C dev... ;-) ).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
str(n)cpy is always tricky. I would do the getenv only once. Then strdup either the result or the default. And free the dupped string at shutdown.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@uli42 Can you help with this MR and fine-polish it? Your C is so much better than mine.
af91cad
to
63adc88
Compare
I am currently limited hw-wise. But I think this looks good except that you
use strncpy in two different ways. You should make them identical regarding
the length argument.
I suggest you test and merge it. You can leave this ticket open and I will
revisit eventually.
…On Wed, Nov 20, 2024 at 9:52 PM Mike Gabriel ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In nxcomp/src/Loop.cpp
<#1079 (comment)>
:
> @@ -14731,7 +14731,19 @@ static void handleAlertInLoop()
{
char caption[DEFAULT_STRING_LENGTH];
- strcpy(caption, ALERT_CAPTION_PREFIX);
+ const char* caption_prefix = getenv("NX_DIALOG_CAPTIONPREFIX");
+
+ if (!caption_prefix) {
+ // Use the hard-coded 'NX -' as default.
+ strncpy(caption, ALERT_CAPTION_PREFIX, DEFAULT_STRING_LENGTH);
+ caption[DEFAULT_STRING_LENGTH-1] = '\0';
+ }
+ else
+ {
+ // Use what gets provided via NX_DIALOG_CAPTIONPREFIX (used in X2Go)
+ strncpy(caption, caption_prefix, DEFAULT_STRING_LENGTH-1);
@uli42 <https://github.com/uli42> Can you help with this MR and
fine-polish it? Your C is so much better than mine.
—
Reply to this email directly, view it on GitHub
<#1079 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABQHBZDCBY4MMIX6S6GJGK32BTY75AVCNFSM6AAAAABKDMRZM2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINBZGY3TKMBUG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
…ixed with a custom prefix string.
63adc88
to
20ddd71
Compare
…ixed with a custom prefix string.