On Mon, 1 Apr 2024 at 17:26, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Ville Skyttä <ville.skytta@xxxxxx> 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".
