All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] win32: ensure that `localtime_r()` is declared even in i686 builds
Date: Mon, 22 Jun 2026 11:11:23 +0200	[thread overview]
Message-ID: <ajj8OylK7jIMhFwp@pks.im> (raw)
In-Reply-To: <pull.2157.git.1782117847057.gitgitgadget@gmail.com>

On Mon, Jun 22, 2026 at 08:44:06AM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The `__MINGW64__` constant is defined, surprise, surprise, only when
> building for a 64-bit CPU architecture.
> 
> Therefore using it as a guard to define `_POSIX_C_SOURCE` (so that
> `localtime_r()` is declared, among other functions) is not enough, we
> also need to check `__MINGW32__`.
> 
> Technically, the latter constant is defined even for 64-bit builds. But
> let's make things a bit easier to understand by testing for both
> constants.

So it would suffice to use `__MINGW32__`? In any case, I agree that
making this explicit feels sane.

> Making it so fixes this compile warning (turned error in GCC v14.1):
> 
>   archive-zip.c: In function 'dos_time':
>   archive-zip.c:612:9: error: implicit declaration of function 'localtime_r';
>   did you mean 'localtime_s'? [-Wimplicit-function-declaration]
>     612 |         localtime_r(&time, &tm);
>         |         ^~~~~~~~~~~
>         |         localtime_s

Makes sense. The function is available in C23, but we don't use it.
Otherwise, it's enabled with `_POSIX_C_SOURCE` according to [1].

> diff --git a/compat/posix.h b/compat/posix.h
> index 2f01564b0d..e2e794cad7 100644
> --- a/compat/posix.h
> +++ b/compat/posix.h
> @@ -56,7 +56,7 @@
>  # define UNUSED
>  #endif
>  
> -#ifdef __MINGW64__
> +#if defined(__MINGW32__) || defined(__MINGW64__)
>  #define _POSIX_C_SOURCE 1
>  #elif defined(__sun__)
>   /*

This looks nice and simple.

Thanks!

Patrick

[1]: https://man7.org/linux/man-pages/man3/ctime.3.html

      reply	other threads:[~2026-06-22  9:11 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-22  8:44 [PATCH] win32: ensure that `localtime_r()` is declared even in i686 builds Johannes Schindelin via GitGitGadget
2026-06-22  9:11 ` Patrick Steinhardt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajj8OylK7jIMhFwp@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.