Skip to content

Add caml_stat_strndup_{to,of}_utf16 to win32 functions #11449

Description

@dra27

The UTF-8/UCS-2 conversion functions assume null-terminated strings on the OS side, but this isn't the case for some functions in the Windows API. In this case, caml_stat_strdup_to_utf16 returns an LPWSTR whose length cannot be determined without making a second call to caml_win32_multi_byte_to_wide_char (since wcslen cannot be used).

I propose adding two additional functions, loosely based around the idea of strndup (another name may be more suitable, as strndup has different semantics w.r.t. null terminators):

/* Converts `len` bytes/characters of `s` between UTF-8 and UTF-16. The pointer `out_len` receives
   the length of the resulting buffer. If `len` is `-1` then `s` is assumed to be null-terminated
   and the entire string is copied including the null-terminator. Unlike strndup(3), these functions
   never add a null terminator. */

wchar_t *caml_stat_strndup_to_utf16(const char *s, size_t len, size_t *out_len);
caml_stat_string caml_stat_strndup_of_utf16(const wchar_t *s, size_t len, size_t *out_len);

/* Converts `len` characters of `s` to an OCaml string. If `s` is null-terminated, use
   `caml_copy_string_of_utf16` instead. Any null characters (including a terminating
   null) will be included in the resulting OCaml value. */
value caml_copy_bytes_of_utf16(const wchar_t *s, size_t len);

I don't think it's necessary to add _os versions of these functions, since I'm not sure how you'd be writing portable code requiring this.

Activity

  1. xavierleroy commented on Jul 21, 2022

    @xavierleroy
    Contributor

    How often non-null-terminated strings occur? It is often possible to null-terminate them before continuing, e.g. instead of

       len = syscall(buffer, sizeof(buffer));
    

    do

      len = syscall(buffer, sizeof(buffer) - 1);
      // handle errors
      buffer[len] = 0;
    
  2. dra27 commented on Jul 21, 2022

    @dra27
    MemberAuthor

    It's not frequently a problem, but it comes up with some I/O functions (e.g. the Registry) - in this case passing a UCS-2 buffer with a length. In this case, we have all the tools in the runtime to go from an OCaml string/bytes (in UTF-8) to a UCS-2 buffer, but win_multi_byte_to_wide_char is both internal and ends up being called twice currently (once by the caller to work out the resulting buffer length and then again by caml_stat_strdup_to_utf16)

  3. samwgoldman commented on Jul 21, 2022

    @samwgoldman
    Contributor

    A possible use case which I encountered recently is FILE_NOTIFY_INFORMATION. ReadDirectoryChangesW writes a sequence of these into a buffer, so inserting a null byte would potentially overwrite.

  4. mroch commented on Jul 21, 2022

    @mroch

    A possible use case which I encountered recently is FILE_NOTIFY_INFORMATION. ReadDirectoryChangesW writes a sequence of these into a buffer, so inserting a null byte would potentially overwrite.

    specifically, here: https://github.com/facebook/flow/blob/main/src/hack_forked/fsnotify_win/fsnotify_stubs.c#L107-L115

    we had to expose win_wide_char_to_multi_byte. at the least, it'd be nice to make that public!

    @dra27 i'm really curious how we both ran into this days apart :)

  5. dra27 commented on Jul 21, 2022

    @dra27
    MemberAuthor

    @dra27 i'm really curious how we both ran into this days apart :)

    Spooky! I'm was (finally) updating opam to support Windows Unicode properly (the C stubs I contributed in 2.0 were written for OCaml 4.02, well before all the support was added in 4.06)

  6. mroch commented on Oct 19, 2022

    @mroch

    Lwt has a bug (ocsigen/lwt#966) whose fix needs access to these functions. An "environment block" consists of a null-terminated block of null-terminated strings.

    ocaml itself deals with this here:

    size =
    caml_win32_multi_byte_to_wide_char(String_val(env),
    caml_string_length(env), NULL, 0);
    wenv = caml_stat_alloc((size + 1)*sizeof(wchar_t));
    caml_win32_multi_byte_to_wide_char(String_val(env),
    caml_string_length(env), wenv, size);

  7. dra27 commented on Oct 20, 2022

    @dra27
    MemberAuthor

    There's an implementation of the first two in one of my relocatable branches (dra27@8fe97fc) - it can either end up going in with that or can be extracted to go in separately.

  8. github-actions commented on Oct 25, 2023

    @github-actions

    This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

  9. reopened this on Aug 7, 2024
  10. added a commit that references this issue on Sep 16, 2024
  11. linked a pull request that will close this issueAdd `caml_stat_char_array_{to,of}_os` #13497on Sep 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions