From: Piotr Caban <piotr.caban@gmail.com>
Subject: Re: msvcrt: add missing tokens in strftime_helper (except %r)
Message-Id: <d0ef1e6d-b88b-5955-5d09-bdf4fbdee40f@gmail.com>
Date: Wed, 2 Oct 2019 17:04:26 +0200
In-Reply-To: <CAHS-jcCcuS4NgBBu4oz=Sdjd8wC8L6shMMy_+N_sUAH1=cn_Xw@mail.gmail.com>
References: <CAHS-jcCcuS4NgBBu4oz=Sdjd8wC8L6shMMy_+N_sUAH1=cn_Xw@mail.gmail.com>

Hi Chuck,

I'll comment on 'h' and 'G' formats that are not implemented in wine 
(and there are no patches for them to be reviewed).
> +        case 'h': >           case 'b':The 'h' format was introduced in ucrtbase. All 
other options that were introduced in newer version of crt library are 
implemented inside
#if _MSVCR_VER >= 140 block.

> +        case 'G':
> +            {
> +#ifdef _WIN64
> +                MSVCRT___time64_t firstmontime = 0;
> +                MSVCRT___time64_t temp_time = 0;
> +#else
> +                MSVCRT___time32_t firstmontime = 0;
> +                MSVCRT___time32_t temp_time = 0;
> +#endif
> +                struct MSVCRT_tm* tmp_mstm = 0;
> +                struct MSVCRT_tm* oldstate = 0;
> +                struct MSVCRT_tm* firstmontm = 0;
> +                int iso8601yr = -1;
> +                int iso8601wk = -1;
> +                char* tmp_str = 0;
> +
> +                if(!(tmp_mstm = MSVCRT_malloc(sizeof(struct MSVCRT_tm))))
> +                  return 0;
> +                if(msvcrt_get_thread_data()->time_buffer &&
> +                       !(oldstate = MSVCRT_malloc(sizeof(struct 
> MSVCRT_tm)))) {
> +                  MSVCRT_free(tmp_mstm);
> +                  return 0;
> +                }
> +
> +                if(oldstate)
> +                    memcpy(oldstate, msvcrt_get_thread_data()->time_buffer,
> +                               sizeof(struct MSVCRT_tm));
> +
> +                memcpy(tmp_mstm, mstm, sizeof(struct MSVCRT_tm));
> +
> +                temp_time = MSVCRT_mktime(tmp_mstm);
> +                MSVCRT_free(tmp_mstm);
> +                tmp_mstm = MSVCRT_gmtime(&temp_time);
> +
> +                iso8601yr = tmp_mstm->tm_year + 1900;
> +                iso8601wk = 
> (tmp_mstm->tm_yday+1-(tmp_mstm->tm_wday==0?7:tmp_mstm->tm_wday)+10)/7;
> +                if(iso8601wk == 53) {
> +                   if(!(firstmontm = MSVCRT_malloc(sizeof(struct 
> MSVCRT_tm)))) {
> +                       MSVCRT_free(oldstate);
> +                       return 0;
> +                   }
> +                   memset(firstmontm, '\0', sizeof(struct MSVCRT_tm));
> +                   firstmontm->tm_mon = 0;
> +                   firstmontm->tm_mday = 4;
> +                   firstmontm->tm_year = tmp_mstm->tm_year+1;
> +                   firstmontime = MSVCRT_mktime(firstmontm);
> +                   MSVCRT_free(firstmontm);
> +                   firstmontm = MSVCRT_gmtime(&firstmontime);
> +                   firstmontime = 
> firstmontime-((firstmontm->tm_wday-1==-1?6:firstmontm->tm_wday-1)*86400);
> +                   if(firstmontime <= temp_time)
> +                      iso8601yr += 1;
> +                }
> +                if(iso8601wk == 0)
> +                  iso8601yr -= 1;
> +
> +                if(!(tmp_str = MSVCRT_malloc(6))) {
> +                    MSVCRT_free(oldstate);
> +                    return 0;
> +                }
> +                memset(tmp_str, '\0', 6);
> +
> +                MSVCRT__snprintf(tmp_str, 6, "%d", iso8601yr);
> +                if(!strftime_str(str, &ret, max, tmp_str)) {
> +                    if(oldstate) {
> +                        memcpy(msvcrt_get_thread_data()->time_buffer, 
> oldstate,
> +                                   sizeof(struct MSVCRT_tm));
> +                        MSVCRT_free(oldstate);
> +                    }
> +                    MSVCRT_free(tmp_str);
> +                    return 0;
> +                }
> +                if(oldstate) {
> +                    memcpy(msvcrt_get_thread_data()->time_buffer, oldstate,
> +                               sizeof(struct MSVCRT_tm));
> +                    MSVCRT_free(oldstate);
> +                }
> +                MSVCRT_free(tmp_str);
> +            }
> +            break;
It's not the simplest way of implementing it. There's no need for any 
allocations, the value returned by this format is in tm_year-1...tm_year 
range+1.

I think that the code should look like this:
tmp = 1900 + mstm->tm_year;
if (mstm->tm_yday - mstm->tm_wday + 4 < 0)
     tmp--;
else if(mstm->tm_yday + mstm->tm_wday - X > 365 + IsLeapYear(tmp))
     tmp++;
if (!strftime_int(str, &ret, max, tmp, 4, 0, 9999))
     return 0;
break;

It also needs some tests.

Thanks,
Piotr