Re: [RFC][PATCH] t-prio-queue: simplify using compound literals

From: René Scharfe <l.s.r@web.de>
Date: Apr 6, 2024, 07:06 UTC
Message-ID: <88f2063e-bddb-4479-a0c1-5bad582b9fe9@web.de>
In-Reply-To: <xmqqedbjtqnr.fsf@gitster.g>

Am 06.04.24 um 00:01 schrieb Junio C Hamano:

Show 15 quoted lines

> Jeff King <peff@peff.net> writes:
>
>> Yeah, this seems pretty reasonable. I think we've traditionally been
>> hesitant to pass or return structs by value, but that's mostly
>> superstition.
>
> We should still be hesitant against the practice to the same degree
> that we are hesitant against struct assignment, especially when the
> struct is of nontrivial size, or the struct has a pointer member
> whose memory ownership semantics goes against shallow copying of the
> struct.
>
> In this particular case, I do not know offhand if .strftime_fmt is
> safe to be shallowly copied, but I trust you two know and/or have
> already looked at the implications.

date_mode_from_type() sets .strftime_fmt to NULL in the struct date_mode it creates and returns. Giving a reference to it to date_mode_release() is a safe NOOP.

show_date() passes .strftime_fmt to strbuf_addftime() and does not retain a copy or change it.

show_ident_date() only passes its struct date_mode parameter to show_date().

format_person_part() only passes its struct date_mode parameter to show_ident_date().

format_reflog_person() only passes its struct date_mode parameter to format_person_part().

get_reflog_selector() only passes its struct date_mode parameter to show_date().

show_reflog_message() only passes its struct date_mode parameter to get_reflog_selector().

The patch doesn't change any other function signatures.

strbuf_addftime() scans the format string and passes it to strbuf_expand_step(), skip_prefix() and strftime(3). None of them change it or retain a copy of the pointer.

In other words: This patch mostly affects the read-only side of struct date_mode handling, and the date_mode_from_type() part is benign as well.

René