Re: [PATCH] [Outreachy] patch-ids: fix NEEDSWORK timezone parsing in fast-import.c

[Patrick Steinhardt <ps@pks.im>]

On Fri, Oct 10, 2025 at 12:49:57AM +0100, Okhuomon Ajayi wrote:
> Signed-off-by: Okhuomon Ajayi <okhuomonajayi54@gmail.com>
> ---

For a change like this it is important to explain what the problem is,
why it is a problem and how your change improves the code for the
better. All of this needs to be patr of the commit message so that the
reader can understand what you're actually doing.

Also, if this fixes a real issue, is it possible to demonstrate the
issue and the fix with a test?

> diff --git a/builtin/fast-import.c b/builtin/fast-import.c
> index 606c6aea82..695e1a0ae1 100644
> --- a/builtin/fast-import.c
> +++ b/builtin/fast-import.c
> @@ -1959,14 +1959,15 @@ static int validate_raw_date(const char *src, struct strbuf *result, int strict)
>  		return -1;
>  
>  	num = strtoul(src + 1, &endp, 10);
> -	/*
> -	 * NEEDSWORK: check for brokenness other than num > 1400, such as
> -	 *            (num % 100) >= 60, or ((num % 100) % 15) != 0 ?
> -	 */
> -	if (errno || endp == src + 1 || *endp || /* did not parse */
> -	    (strict && (1400 < num))             /* parsed a broken timezone */
> -	   )
> +	
> +
> +        unsigned int hours = num / 100;
> +        unsigned int minutes = num % 100;
> +
> +	if (errno || endp == src + 1 || *endp || 
> +	    (strict && (num > 1400 || minutes >=60 || minutes % 15 != 0))){
>  		return -1;
> +	}

Despite the formatting issues I also think that this here is becoming
hard to read. It may make sense to split this up into multiple
conditions.

Thanks!

Patrick