Skip to content

cygwin.c: fix several silly/terrible C errors #22724

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

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

mauke
Copy link
Contributor

@mauke mauke commented Nov 6, 2024

Errors fixed:

Not detected by the compiler, but the preprocessor version checks only looked at CYGWIN_VERSION_API_MINOR, ignoring
CYGWIN_VERSION_API_MAJOR.

cygwin.c: In function 'utf8_to_wide_extra_len':
cygwin.c:187:18: warning: initialization of 'size_t' {aka 'long unsigned int'} from 'size_t *' {aka 'long unsigned int *'} makes integer from pointer without a cast [-Wint-conversion]
  187 |     Size_t len = strlen(buf) + extra_len + 1;
      |                  ^~~~~~

extra_len was declared as a pointer, but used as an integer. I don't see how this could have ever worked.

cygwin.c: In function 'S_convert_path_common':
cygwin.c:339:38: warning: 'realloc' called on unallocated object 'wconverted' [-Wfree-nonheap-object]
  339 |             wconverted = (wchar_t *) realloc(&wconverted, newlen);
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
cygwin.c:357:39: warning: 'realloc' called on unallocated object 'converted_path' [-Wfree-nonheap-object]
  357 |             converted_path = (char *) realloc(&converted_path, newlen);
      |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Again, I don't see how this could have ever worked. We're passing the address of a local (stack) variable to realloc (instead of the value of the variable).

cygwin.c: In function 'wide_to_utf8':
cygwin.c:173:32: warning: pointer targets in passing argument 3 of 'Perl_utf16_to_utf8_base' differ in signedness [-Wpointer-sign]
  173 |     utf16_to_utf8((U8 *) wsrc, buf, wlen, &blen);
      |                                ^~~
      |                                |
      |                                char *

The argument is declared as U8 *, so this is mostly harmless, but a cast silences the warning anyway.

cygwin.c: In function 'utf8_to_wide_extra_len':
cygwin.c:194:19: warning: passing argument 2 of 'Perl_utf8_to_utf16_base' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  194 |     utf8_to_utf16(buf, (U8 *) wsrc, len, &wlen);
      |                   ^~~

This is arguably a bug in utf8_to_utf16, which doesn't declare its input argument as pointer-to-const. But it's also a char * vs U8 * type mismatch, so a cast silences both issues.

cygwin.c: In function 'S_convert_path_common':
cygwin.c:292:22: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
  292 |         char *name = (direction == to_posix)
      |                      ^

This is a missing const in the declaration of a variable that points to string literals.


  • This set of changes does not require a perldelta entry.

Errors fixed:

Not detected by the compiler, but the preprocessor version checks only
looked at `CYGWIN_VERSION_API_MINOR`, ignoring
`CYGWIN_VERSION_API_MAJOR`.

    cygwin.c: In function 'utf8_to_wide_extra_len':
    cygwin.c:187:18: warning: initialization of 'size_t' {aka 'long unsigned int'} from 'size_t *' {aka 'long unsigned int *'} makes integer from pointer without a cast [-Wint-conversion]
      187 |     Size_t len = strlen(buf) + extra_len + 1;
          |                  ^~~~~~

`extra_len` was declared as a pointer, but used as an integer. I don't
see how this could have ever worked.

    cygwin.c: In function 'S_convert_path_common':
    cygwin.c:339:38: warning: 'realloc' called on unallocated object 'wconverted' [-Wfree-nonheap-object]
      339 |             wconverted = (wchar_t *) realloc(&wconverted, newlen);
          |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
    cygwin.c:357:39: warning: 'realloc' called on unallocated object 'converted_path' [-Wfree-nonheap-object]
      357 |             converted_path = (char *) realloc(&converted_path, newlen);
          |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Again, I don't see how this could have ever worked. We're passing the
address of a local (stack) variable to `realloc` (instead of the value
of the variable).

    cygwin.c: In function 'wide_to_utf8':
    cygwin.c:173:32: warning: pointer targets in passing argument 3 of 'Perl_utf16_to_utf8_base' differ in signedness [-Wpointer-sign]
      173 |     utf16_to_utf8((U8 *) wsrc, buf, wlen, &blen);
          |                                ^~~
          |                                |
          |                                char *

The argument is declared as `U8 *`, so this is mostly harmless, but a
cast silences the warning anyway.

    cygwin.c: In function 'utf8_to_wide_extra_len':
    cygwin.c:194:19: warning: passing argument 2 of 'Perl_utf8_to_utf16_base' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      194 |     utf8_to_utf16(buf, (U8 *) wsrc, len, &wlen);
          |                   ^~~

This is arguably a bug in `utf8_to_utf16`, which doesn't declare its
input argument as pointer-to-const. But it's also a `char *` vs `U8 *`
type mismatch, so a cast silences both issues.

    cygwin.c: In function 'S_convert_path_common':
    cygwin.c:292:22: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      292 |         char *name = (direction == to_posix)
          |                      ^

This is a missing `const` in the declaration of a variable that points
to string literals.
@mauke mauke merged commit b872e74 into Perl:blead Nov 7, 2024
@mauke mauke deleted the fix-silly-cygwin-errors branch November 7, 2024 05:50
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

Successfully merging this pull request may close these issues.

2 participants