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

Switch usages of strlcpy and strlcat to more cross-compatible methods #9

Open
JamesHagerman opened this issue Sep 4, 2020 · 3 comments

Comments

@JamesHagerman
Copy link

When compiling this under Windows Subsystem for Linux (WSL) using Ubuntu, I was able to get this to compile using the following steps:

  1. Install the build tool chain and the dependencies:

sudo apt install build-essential bison flex

  1. Patch nqc/DirList.cpp to replace strlcpy() and strlcat() with strncpy() and strcat() respectively. Usage of sizeof() worked to at least compile bin/nqc. Here's the git diff showing the patched lines. (Note: I'm not sure this is the right way, but it should be close... I think???)
diff --git a/nqc/DirList.cpp b/nqc/DirList.cpp
index 766b2b4..6bccc93 100644
--- a/nqc/DirList.cpp
+++ b/nqc/DirList.cpp
@@ -53,7 +53,7 @@ bool DirList::Find(const char *filename, char *pathname)
     struct stat stat_buf;

     size_t len = sizeof(pathname);
-    if (strlcpy(pathname, filename, len) >= len) {
+    if (sizeof(strncpy(pathname, filename, len)) >= len) {
         return false;
     }

@@ -61,8 +61,8 @@ bool DirList::Find(const char *filename, char *pathname)
         return true;

     for(Entry *e = fEntries.GetHead(); e; e=e->GetNext()) {
-        if (strlcpy(pathname, e->GetPath(), len) < len) {
-            if (strlcat(pathname, filename, len) < len) {
+        if (sizeof(strncpy(pathname, e->GetPath(), len)) < len) {
+            if (sizeof(strncat(pathname, filename, len)) < len) {
                 if (stat(pathname, &stat_buf) == 0) {
                     return true;
                 }
  1. Run make from the root of the repository
  2. Run bin/nqc

If this sounds acceptable, I'll open a PR for the change.

@jverne
Copy link
Owner

jverne commented Oct 3, 2022

It probably used strncpy and strcat historically before I introduced the "safer" versions, so you should be able to check history. I never really completed the conversion to this API, but ideally this would be handled as a cross-platform compile config rather than reverting the use completely. Esepcially since those two routines are in the project already.

If I get some time I'll take a look at finishing the conversion and either abandoning it or adding the glue to make it work on GNUish platforms.

@davidperrenoud
Copy link

You can upstream those changes:
https://github.com/BrickBot/nqc/blob/master/nqc/DirList.cpp

They seem to work fine on Ubuntu 22.04.

@mesheets
Copy link

Hello from the BrickBot org! Linux builds are supported in the codebase there (including a GitHub action set to run on Linux), and NQC-related packages are available for the Slitaz Linux OS. Glad to hear of the good report regarding Ubuntu!

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

No branches or pull requests

4 participants