Skip to content

Use strlcpy() in xrdp_init_xkb_layout() #3488

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 3, 2025
Merged

Use strlcpy() in xrdp_init_xkb_layout() #3488

merged 2 commits into from
Apr 3, 2025

Conversation

matt335672
Copy link
Member

@matt335672 matt335672 commented Mar 31, 2025 •
edited
Loading

Replace uses of g_strncpy() in xrdp_init_xkb_layout() with the more correct strlcpy().

In addition, some values and pointers which do not need to be writeable have been made const.

Draft for now, as I'm out of time to test this today.

@matt335672 matt335672 requested a review from Copilot April 1, 2025 09:22
Copilot
Copilot AI reviewed Apr 1, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • xrdp/lang.c: Language not supported

@matt335672
Use strlcpy() in xrdp_init_xkb_layout()
f1f2ce5 
Replace uses of g_strncpy() in xrdp_init_xkb_layout() with the more
correct strlcpy().

In addition, some values and pointers which do not need to be
writeable have been made const.
@matt335672
xrdp_keyboard.ini : Provide defaults
38b1d65 
At present, in the unlikely event the xrdp_keyboard.ini file is damaged,
sensible defaults are not provided for the values in the [defaults]
section.
@matt335672 matt335672 marked this pull request as ready for review April 1, 2025 10:52
@matt335672
Copy link
Member Author

So far I'm not terrifically impressed with Copilot's review capabilities. It seems unable to determine that lang.c is a C language source file. When I asked it about this I got:-

The pull request #3488 in the neutrinolabs/xrdp repository includes changes to the xrdp/lang.c file. The message "Language not supported" generated by Copilot might have been due to a temporary issue or misconfiguration. The file xrdp/lang.c is written in C, which is a supported language.

If you continue to encounter this message, it might be useful to check the Copilot configuration or contact GitHub support for further assistance. If you need any specific help with the changes in this file, feel free to ask!

@metalefty
Copy link
Member

Changes LGTM. I'm fine with merging this at the moment.

As a little bit long-term thoughts, let's proceed to TOML. Newly added configuration files should be TOML, so we implemented GFX configurations using TOML.

The next step is to switch existing configuration files to TOML. We already switched keymap files to TOML so now it is time for keyboard configuration. I suppose also rsakeys.ini can be switched to a more standard format such as PEM. xrdp.ini and sesman.ini are big ones so they will be the last.

@matt335672
Copy link
Member Author

Agreed.

The keymap should be pretty straightforward. rsakeys.ini could be converted to PEM, but that would need a bit of work - it's not an area I'm familiar with.

The big two will probably need some migration utilities so people can talk old format files and produce toml outputs.

@matt335672 matt335672 merged commit 6b5c635 into neutrinolabs:devel Apr 3, 2025
26 of 27 checks passed
@matt335672 matt335672 deleted the modernise_lang_c branch April 3, 2025 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@matt335672 @metalefty