Skip to content

Conversation

davidpanderson
Copy link
Contributor

@Copilot Copilot AI review requested due to automatic review settings April 11, 2025 21:09
Copy link
Contributor

@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 reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment on lines 2212 to 2213
// (big mess, permssions problems, doesn't generally work)
// new approach: get last-input time from a separate daemon process,
Copy link
Preview

Copilot AI Apr 11, 2025

Choose a reason for hiding this comment

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

Typo in the comment: 'permssions' should be corrected to 'permissions'.

Suggested change
// (big mess, permssions problems, doesn't generally work)
// new approach: get last-input time from a separate daemon process,
// (big mess, permissions problems, doesn't generally work)

Copilot uses AI. Check for mistakes.


uint64_t input_time = seg_ptr[1];
idle_time = gstate.now - input_time;
printf("idle time: %ld\n", idle_time);
Copy link
Preview

Copilot AI Apr 11, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using the project's logging mechanism instead of printf for production logging to ensure consistency and proper log level management.

Copilot uses AI. Check for mistakes.

…ction.

shm_open() and mmap() are the standard way to do this.

This requires -lrt.
I just added this to Makefile.am; probably not the right way.
@jamescowens
Copy link

jamescowens commented Apr 12, 2025

@davidpanderson do you mind making these signed 64 bit ints instead of unsigned so that we conform to the new time standard?

POSIX requires time to be a signed int. The complete spec in <time.h> is actually

struct timespec {
    time_t   tv_sec;        /* seconds */
    long     tv_nsec;       /* nanoseconds */
};

Where time_t is typedefd internally in the header file to a 64 bit signed integer type on that platform.

We obviously don't need the nanoseconds part for this so just using the 64 bit signed integer is perfectly fine.

@davidpanderson
Copy link
Contributor Author

Uh, sure.

@AenBleidd
Copy link
Member

This PR fails during the build for Android:

hostinfo_unix.cpp:567:52: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare]
            strlcpy(buf2, strchr(buf, ':') + 2, ((t<sizeof(buf2))?t:sizeof(buf2)));
                                                  ~^~~~~~~~~~~~~
hostinfo_unix.cpp:2219:18: error: use of undeclared identifier 'shm_open'
        int fd = shm_open("/idle_detect_shmem",  O_RDONLY, 0);
                 ^
1 warning and 1 error generated.

@davidpanderson
Copy link
Contributor Author

fixed (I hope)

- include -lcrypto after -lssl
- include -lrt only if it exists
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment