On Thu, Mar 31, 2016 at 12:31 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Wed, Mar 30, 2016 at 8:17 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: >> The calling shell code makes sure that `path` is non null and non empty. >> (side note: it cannot be null as just three lines before it is passed >> to safe_create_leading_directories_const which would crash as you feed >> it null). > > I'm confused by this. So, you're saying that it's okay (and desirable) > for git-submodule--helper to segfault when the user does this: > > % git submodule--helper clone > Segmentation fault: 11 > > rather than, say, printing a useful error message, such as: > > submodule--helper: missing or empty --path I think I was rather saying, * that you see the segfault behavior not at all when channeling the command through the proper frontend command * that it already crashes (we access *path before this check, so this check is pointless) > While one can argue that this is an > internal command only invoked in a controlled fashion, it's not hard > to imagine someone running it manually to understand its behavior or > to debug some problem. (additionally:) And later we may want to reuse this code when replacing the frontend command to be written in C completely. > if (!path || !*path) > die(_("submodule--helper: unspecified or empty --path")); That sounds good to me. I'll pick it up. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html