Skip to content

fix clang warnings that are bugs #3917

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

Merged
ptitSeb merged 3 commits into from
May 29, 2026
Merged

fix clang warnings that are bugs #3917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5cea290
remove suffix only if present
peppergrayxyz May 29, 2026
9ecf742
fix isProcAny check
peppergrayxyz May 29, 2026
8c829be
add missing paramter
peppergrayxyz May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/tools/env.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,8 +783,9 @@ void RecordEnvMappings(uintptr_t addr, size_t length, int fd)
if(strstr(lowercase_filename, "/memfd:")==lowercase_filename) {
// memfd, first remove the (deleted) at the end
char* p = strstr(lowercase_filename, " (deleted)");
if(p=lowercase_filename+strlen(lowercase_filename)-strlen(" (deleted)"))
*p = 0;
if(p == lowercase_filename+strlen(lowercase_filename)-strlen(" (deleted)")) {
*p = '\0';
}
// add the "/fd" at the end to differenciate between memfd
char* new_name = box_calloc(1, strlen(lowercase_filename)+100);
sprintf(new_name, "%s/%d", lowercase_filename, fd);
Expand Down
6 changes: 3 additions & 3 deletions src/wrapped/wrappedlibc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2043,8 +2043,8 @@ static int isProcAny(const char *path, const char* w)
if(strncmp(path, "/proc/", 6)==0) {
int pid;
char p[4096] ={0};
if(sscanf(path, "/proc/%d/%s", &pid, &p)==2)
if(p && !strcmp(p, w))
if(sscanf(path, "/proc/%d/%s", &pid, p)==2)
if(strcmp(p, w) == 0)
Copy link
Copy Markdown
Owner

@ptitSeb ptitSeb May 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The change from !strcmp(...) tp strcmp(...) == 0 smells a lot like A.I. A human would have just removed the leading p && ...

(the comments of the PR/Tickets are also very verbose)

Just a reminder, we ask everyone to declare if they use A.I for a PR, and not to let A.I. do all the comunication (creating tickets), as we are human handling the tickets.

Also also, it's always nice to see wich agent are use to do stuffs.

Copy link
Copy Markdown
Contributor Author

@peppergrayxyz peppergrayxyz May 29, 2026

Choose a reason for hiding this comment

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

The form strcmp(...) == 0 is used on the lines just below (L2059-64), so I thought I'll keep the style consistent:

  if(strcmp((const char*)path, tmp)==0)
      return 1;
  // check if self PID ....
  pid_t pid = getpid();
  sprintf(tmp, "/proc/%d/%s", pid, w);
  if(strcmp((const char*)path, tmp)==0)

I used to get feedback on my PRs that I'm too minimalistic, so I started being more verbose. Also I'm on arm64 Gentoo/musl/libcxx which is usually not supported by projects, so I try to make it easy for maintainers in the hopes to get them more likely upstreamed.

Sorry for not acting human enough 😅

Copy link
Copy Markdown
Owner

@ptitSeb ptitSeb May 29, 2026

Choose a reason for hiding this comment

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

Ah well, sorry, I ted to be more and more paranoiac...

Oh, musl, interesting...

Copy link
Copy Markdown
Contributor Author

@peppergrayxyz peppergrayxyz May 29, 2026

Choose a reason for hiding this comment

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

No worries. Crazy times...

I'm not familiar with the source base, so I'm curious if I can get musl to work or hit a roadblock. But so far it looks good and Android doesn't use glibc either, so it shouldn't be I'm cautiously optimistic 🤞

Copy link
Copy Markdown
Owner

@ptitSeb ptitSeb May 29, 2026

Choose a reason for hiding this comment

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

Well, there is no specific musl support, but having a build that does run musl x64 binry should be possible without too much trouble... Running glibc x64 binary is another story tho....

return pid;
}
return -1;
Expand Down Expand Up @@ -2125,7 +2125,7 @@ EXPORT ssize_t my_readlink(x64emu_t* emu, void* path, void* buf, size_t sz)
if(filename[0]=='.') {
// relative path, need to grap cwd and cannonicalise the path
char cwd_name[strlen(path)+4];
sprintf(cwd_name, "/proc/%d/cwd");
sprintf(cwd_name, "/proc/%d/cwd", pid);
char cwd[MAX_PATH] = {0};
if(readlink(cwd_name, cwd, MAX_PATH)>0 && strlen(cwd)+strlen(path)+1<MAX_PATH) {
strcat(cwd, "/");
Expand Down
Loading