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

[bug]: recent change refers to ALLPERMS which is not always defined #149

Closed
1 task done
micmac1 opened this issue Jun 5, 2023 · 3 comments · Fixed by #150
Closed
1 task done

[bug]: recent change refers to ALLPERMS which is not always defined #149

micmac1 opened this issue Jun 5, 2023 · 3 comments · Fixed by #150
Assignees
Labels
bug support-level-core Functionality with core support level

Comments

@micmac1
Copy link
Contributor

micmac1 commented Jun 5, 2023

Severity

Minor

Versions

20.2.0

Components/Modules

res_crypto

Operating Environment

OpenWrt musl libc

Frequency of Occurrence

Constant

Issue Description

Commit d0bea5a included sys/stat.h and refers to ALLPERMS, which is not POSIX and as such may not be available everywhere.

Would be nice to get this fixed, thanks.

Ping @pprindeville

Relevant log output

res_crypto.c: In function 'try_load_key':
res_crypto.c:222:28: error: 'ALLPERMS' undeclared (first use in this function)
  222 |             ((st.st_mode & ALLPERMS) & ~(S_IRUSR | S_IWUSR)) != 0) {
      |                            ^~~~~~~~

Asterisk Issue Guidelines

  • Yes, I have read the Asterisk Issue Guidelines
@seanbright seanbright self-assigned this Jun 5, 2023
@jcolp jcolp added support-level-core Functionality with core support level and removed triage labels Jun 5, 2023
micmac1 added a commit to micmac1/telephony that referenced this issue Jun 7, 2023
- bump to 20.3.0
- new modules: app-broadcast, app-if, app-signal, func-export,
  res-pjsip-aoc and res-pjsip-rfc3329
- remove "--without-vpb", not available anymore
- add configuration file for res-http-media-cache
- drop libsrtp2 from res-pjproject dependencies, see changes in
  pjproject package
- refresh patches
- add upstream patch
  180-res_crypto.c-Avoid-using-the-non-portable-ALLPERMS-m.patch to fix
  build [1]

[1] asterisk/asterisk#149

Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net>
asterisk-org-access-app bot pushed a commit that referenced this issue Jun 12, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.
asterisk-org-access-app bot pushed a commit that referenced this issue Jun 12, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.
asterisk-org-access-app bot pushed a commit that referenced this issue Jun 12, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.
@pprindeville
Copy link
Contributor

pprindeville commented Jun 12, 2023

Sorry for not having a chance to get to this before: my development testbed had vapors and I spent the last couple of days migrating VMs to a new platform.

Personally, I'd have preferred a low touch fix instead, and since ALLPERMS is a familiar paradigm for most Linux programmers, I'd have left it. The solution I would have gone with is:

#if !defined(ALLPERMS)
#define ALLPERMS ...
#endif

instead.

@seanbright
Copy link
Contributor

seanbright commented Jun 12, 2023

Sorry for not having a chance to get to this before: my development testbed had vapors and I spent the last couple of days migrating VMs to a new platform.

Personally, I'd have preferred a low touch fix instead, and since ALLPERMS is a familiar paradigm for most Linux programmers, I'd have left it. The solution I would have gone with is:

#if !defined(ALLPERMS)
#define ALLPERMS ...
#141

instead.

Feel free to open another PR with your preferred fix.

@pprindeville
Copy link
Contributor

Feel free to open another PR with your preferred fix. Make sure to use #endif instead of #141 though.

Overzealous completion... not sure why it does that inside of triple-backticks...

asterisk-org-access-app bot pushed a commit that referenced this issue Jun 30, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.

(cherry picked from commit 0642f4c)
asterisk-org-access-app bot pushed a commit that referenced this issue Jun 30, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.

(cherry picked from commit 0642f4c)
asterisk-org-access-app bot pushed a commit that referenced this issue Jun 30, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.

(cherry picked from commit 0642f4c)
asterisk-org-access-app bot pushed a commit that referenced this issue Jun 30, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.

(cherry picked from commit 0642f4c)
asterisk-org-access-app bot pushed a commit that referenced this issue Jun 30, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.

(cherry picked from commit 0642f4c)
asterisk-org-access-app bot pushed a commit that referenced this issue Jun 30, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.

(cherry picked from commit ebc0073)
micmac1 added a commit to micmac1/telephony that referenced this issue Jul 9, 2023
- bump to 20.3.0
- new modules: app-broadcast, app-if, app-signal, func-export,
  res-pjsip-aoc and res-pjsip-rfc3329
- remove "--without-vpb", not available anymore
- add configuration file for res-http-media-cache
- drop libsrtp2 from res-pjproject dependencies, see changes in
  pjproject package
- refresh patches
- add upstream patch
  180-res_crypto.c-Avoid-using-the-non-portable-ALLPERMS-m.patch to fix
  build [1]

[1] asterisk/asterisk#149

Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net>
(cherry picked from commit 945b7ea)
asterisk-org-access-app bot pushed a commit that referenced this issue Jul 10, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.

(cherry picked from commit 0642f4c)
asterisk-org-access-app bot pushed a commit that referenced this issue Jul 10, 2023
ALLPERMS is not POSIX and it's trivial enough to not jump through
autoconf hoops to check for it.

Fixes #149.

(cherry picked from commit ebc0073)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug support-level-core Functionality with core support level
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants