From: Ville Skyttä Date: Mon, 01 Apr 2024 19:10:30 GMT Subject: Re: [PATCH] completion: fix prompt with unset SHOWCONFLICTSTATE in nounset mode Message-ID: In-Reply-To: On Mon, 1 Apr 2024 at 17:26, Junio C Hamano wrote: > > Ville Skyttä 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".