From: Junio C Hamano Date: Wed, 01 Jul 2015 16:36:24 GMT Subject: Re: [PATCH v5 3/4] status: give more information during rebase -i Message-ID: In-Reply-To: Eric Sunshine writes: > I was about to mention the same shortcoming, but you beat me to it. > >> Perhaps be more strict and do this instead (without >> leading strbuf_trim): >> >> if (!get_sha1_hex(split[1]->buf, sha1) && >> !strcmp(split[1]->buf + 40, " ") { >> replace split[1] with "%s " abbrev >> } > > Isn't it dangerous to assume that you can index 40 characters into > split[1]? If (for some reason), the user botched the todo line such > that the SHA1 is no longer a valid hex string, then split[1] will be > that botched string, which might be shorter than 40 characters. For > instance, if the user-edited todo line is: > > pick oops nothing > > then git-rebase--interactive.sh:transform_todo_ids() will leave "oops" > as-is, since it can't unabbreviate it, and then this code will place > "oops" into split[1]. Yeah, that is why get_sha1_hex() is checked before we try to make sure buf[40] has " " in the code snippet you quoted. Isn't that how && short-cut works?