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

tests: Fix test FormatDateTime with invalid formats on macOS/*BSD #10149

Merged
merged 1 commit into from
Sep 10, 2024

Conversation

yhabteab
Copy link
Member

@yhabteab yhabteab commented Sep 10, 2024

strftime(3) seems to return the provided invalid format specifiers on all Platforms, i.e even on macOS, but the macOS/*BSD libc implementations of strftime(3) appears to behave differently and causes the % character not to be populated into the results buffer if invalid format specifiers such as "x %! y" are given. If such specifiers are provided, the output will contain x ! y instead of the given invalid format specifiers.

See https://opensource.apple.com/source/Libc/Libc-1439.40.11/stdtime/FreeBSD/strftime.c.auto.html

@yhabteab yhabteab added the area/tests Unit and environment tests label Sep 10, 2024
@yhabteab yhabteab requested a review from oxzi September 10, 2024 09:11
@cla-bot cla-bot bot added the cla/signed label Sep 10, 2024
@yhabteab yhabteab added this to the 2.15.0 milestone Sep 10, 2024
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

First, thanks for fixing the tests on certain platforms.

However, this is not a macOS bug, but a difference in the strftime implementation between different libcs. I created a small test program for x %! y and got the same output on a Linux with glibc and the "shorter output" x ! y on an OpenBSD.

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

int main(void) {
  char *buf;
  time_t t;
  struct tm *tmp;

  t = time(NULL);
  if (! (tmp = localtime(&t))) {
    perror("localtime");
    return EXIT_FAILURE;
  }

  if (! (buf = malloc(BUFSIZ))) {
    perror("malloc");
    return EXIT_FAILURE;
  }

  if (! (strftime(buf, BUFSIZ, "x %! y", tmp))) {
    perror("strftime");
    return EXIT_FAILURE;
  }

  printf("out: \"%s\"\n", buf);
  free(buf);
}
$ uname -sr
OpenBSD 7.6
$ cc -Wall -Werror -pedantic a.c
$ ./a.out
out: "x ! y"
$ uname -sr
Linux 6.10.8
$ cc -Wall -pedantic a.c
a.c: In function ‘main’:
a.c:21:36: warning: unknown conversion type character ‘!’ in format [-Wformat=]
   21 |   if (! (strftime(buf, BUFSIZ, "x %! y", tmp))) {
      |                                    ^
$ ./a.out
out: "x %! y"

Based on strftime from POSIX.1-2017, "[i]f a conversion specification does not correspond to any of the above, the behavior is undefined."

Thus, don't put the blame on 🍎 and please reword this PR and allow the other implementation chosen by macOS and OpenBSD.

@yhabteab yhabteab force-pushed the fix-format-datetime-tests-on-darwin branch from 838d20d to fa38f47 Compare September 10, 2024 14:53
@yhabteab yhabteab requested a review from oxzi September 10, 2024 14:53
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

Please rename the variable - s/darwinBug/percentLessOutput/g or the like - and change the commit message.

@yhabteab yhabteab force-pushed the fix-format-datetime-tests-on-darwin branch from fa38f47 to b8932e6 Compare September 10, 2024 15:11
@yhabteab yhabteab requested a review from oxzi September 10, 2024 15:11
@oxzi oxzi changed the title tests: Fix test FormatDateTime with invalid formats on MacOS tests: Fix test FormatDateTime with invalid formats on macOS/*BSD Sep 10, 2024
@yhabteab yhabteab merged commit 8beb0b7 into master Sep 10, 2024
26 checks passed
@yhabteab yhabteab deleted the fix-format-datetime-tests-on-darwin branch September 10, 2024 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tests Unit and environment tests cla/signed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants