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

Overhaul Utility::FormatDateTime() #10112

Merged
merged 6 commits into from
Aug 26, 2024
Merged

Overhaul Utility::FormatDateTime() #10112

merged 6 commits into from
Aug 26, 2024

Commits on Aug 22, 2024

  1. Configuration menu
    Copy the full SHA
    090dcfd View commit details
    Browse the repository at this point in the history

Commits on Aug 23, 2024

  1. Utility::FormatDateTime(): use boost::numeric_cast<>()

    The previous implementation actually had undefined behavior when called with a
    double that can't be represented as time_t. With boost::numeric_cast, there's a
    convenient cast available that avoids this and throws an exceptions on
    overflow.
    
    It's undefined behavior ([0], where the implicit conversion rule comes into
    play because the C-style cast uses static_cast [1] which in turn uses the
    imlicit conversion as per rule 5 of [2]):
    
    > A prvalue of floating-point type can be converted to a prvalue of any integer
    > type. The fractional part is truncated, that is, the fractional part is
    > discarded.
    >
    > * If the truncated value cannot fit into the destination type, the behavior
    >   is undefined (even when the destination type is unsigned, modulo arithmetic
    >   does not apply).
    
    Note that on Linux amd64, the undefined behavior typically manifests itself in
    the result being the minimal value of time_t which then results in localtime_r
    failing with EOVERFLOW.
    
    [0]: https://en.cppreference.com/w/cpp/language/implicit_conversion#Floating.E2.80.93integral_conversions
    [1]: https://en.cppreference.com/w/cpp/language/explicit_cast
    [2]: https://en.cppreference.com/w/cpp/language/static_cast
    julianbrost committed Aug 23, 2024
    Configuration menu
    Copy the full SHA
    704acdc View commit details
    Browse the repository at this point in the history
  2. Utility::FormatDateTime(): use localtime_s() on Windows

    localtime() is not thread-safe as it returns a pointer to a shared tm struct.
    Everywhere except on Windows, localtime_r() is used already which avoids the
    problem by using a struct allocated by the caller for the output.
    
    Windows actually has a similar function called localtime_s() which has the same
    properties, just with a different name and order of arguments.
    julianbrost committed Aug 23, 2024
    Configuration menu
    Copy the full SHA
    c2c6690 View commit details
    Browse the repository at this point in the history
  3. Utility::FormatDateTime(): handle errors from strftime()

    So far, the return value of strftime() was simply ignored and the output buffer
    passed to the icinga::String constructor. However, there are error conditions
    where strftime() returns 0 to signal an error, like if the buffer was too small
    for the output. In that case, there's no guarantee on the buffer contents and
    reading it can result in undefined behavior. Unfortunately, returning 0 can
    also indicate success and strftime() doesn't set errno, so there's no reliable
    way to distinguish both situations. Thus, the implementation now returns the
    empty string in both cases.
    
    I attempted to use std::put_time() at first as that allows for better error
    handling, however, there were problems with the implementation on Windows (see
    inline comment), so I put that plan on hold at left strftime() there for the
    time being.
    julianbrost committed Aug 23, 2024
    Configuration menu
    Copy the full SHA
    0285028 View commit details
    Browse the repository at this point in the history
  4. Utility::FormatDateTime(): handle invalid format strings on Windows

    On Windows, the strftime() function family invokes an invalid parameter handler
    when the format string is invalid (see the "Remarks" section in their
    documentation). std::put_time() shows the same behavior as it uses
    _wcsftime_l() internally. The default invalid parameter handler may terminate
    the process, which can be a problem given that the format string can be
    specified by the user from the Icinga DSL.
    
    Thus, temporarily set a thread-local no-op handler to disable the default one
    allowing the program to continue. This then simply results in the function
    returning an error which then results in an exception as we ask the stream to
    throw one.
    
    See also:
    https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/strftime-wcsftime-strftime-l-wcsftime-l?view=msvc-170
    https://learn.microsoft.com/en-us/cpp/c-runtime-library/parameter-validation?view=msvc-170
    https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/set-invalid-parameter-handler-set-thread-local-invalid-parameter-handler?view=msvc-170
    julianbrost committed Aug 23, 2024
    Configuration menu
    Copy the full SHA
    d5b3ffa View commit details
    Browse the repository at this point in the history
  5. Utility::FormatDateTime(): provide an overload for tm*

    This allows the function to be used both with a double timestamp or a pointer
    to a tm struct. With this, a similar implementation inside the tests can simply
    use our regular function.
    julianbrost committed Aug 23, 2024
    Configuration menu
    Copy the full SHA
    39ae2e8 View commit details
    Browse the repository at this point in the history