Re: [PATCH] completion: fix prompt with unset SHOWCONFLICTSTATE in nounset mode
- From
Ville Skyttä <ville.skytta@iki.fi>
- Date
- Apr 1, 2024, 19:10 UTC
- Message-ID
- <CABr9L5CLNCYFcta0aMApkyxr_jW9wycY8GmJsUPr9kg1smv+Kg@mail.gmail.com>
- In-Reply-To
- <xmqqh6gl9eq5.fsf@gitster.g>
On Mon, 1 Apr 2024 at 17:26, Junio C Hamano <gitster@pobox.com> wrote:
Show 21 quoted lines
> > Ville Skyttä <ville.skytta@iki.fi> writes: > > > I initially actually changed those very lines too when working on the > > fix for the issue I faced with GIT_PS1_SHOWCONFLICTSTATE. However, > > both occurrences are within __git_ps1_show_upstream, and the only call > > site for that function is protected by a check on the variable that > > does take possible unset state into account; the function will in the > > file's current form never be called with it unset. Additionally, the > > first occurrence is immediately following a line that sets the > > variable, so that one is "doubly protected". > > > > Therefore, I decided to undo those changes and not include them here. > > I guess it's a matter of taste whether one finds it desirable to > > protect those accesses nevertheless, but it's not strictly necessary. > > I am glad you took a look into it already. I wonder if we can > somehow keep this "institutional knowledge" to help the next person > by saving them from wasting time wondering about the reason why it > is safe (iow, what you have found out and described above). Perhaps > a patch like this? I dunno.
TBH, I'd personally just rather patch the "vulnerable" reference to guard against this. Sent that approach in another mail with subject "[PATCH] completion: protect prompt against unset SHOWUPSTREAM in nounset mode".