Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-28 Thread erik elfström
On Tue, Apr 28, 2015 at 8:17 AM, Jeff King p...@peff.net wrote: There was a discussion not too long ago on strategies for returning errors, and one of the suggestions was to return an error strbuf rather than a code[1]. That's less flexible, as the caller can't react differently based on the

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-28 Thread Junio C Hamano
Jeff King p...@peff.net writes: Yes, I agree converting the integer back into a string later does not always carry all of the data. OTOH, the caller can often supply the context (i.e., this is basically how errno works). This gets back to the idea we discussed a while ago of having a struct

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-28 Thread Jonathan Nieder
Jeff King wrote: On Tue, Apr 28, 2015 at 10:07:43PM +0200, erik elfström wrote: Also if it turns out that we actually need to treat the file too large error differently in clean (as discussed in thread on the file size check) then we can no longer communicate that back using the strbuf

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-28 Thread Jonathan Nieder
Jeff King wrote: On Tue, Apr 28, 2015 at 01:42:13PM -0700, Jonathan Nieder wrote: Jeff King wrote: But the NULL does not carry the information about _which_ error, and Erik is suggesting that the caller may need to change behavior based on that information. IOW, his current patch (return

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-28 Thread Jeff King
On Tue, Apr 28, 2015 at 01:34:00PM -0700, Jonathan Nieder wrote: Jeff King wrote: On Tue, Apr 28, 2015 at 10:07:43PM +0200, erik elfström wrote: Also if it turns out that we actually need to treat the file too large error differently in clean (as discussed in thread on the file size

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-28 Thread Jonathan Nieder
Jeff King wrote: But the NULL does not carry the information about _which_ error, and Erik is suggesting that the caller may need to change behavior based on that information. IOW, his current patch (return NULL and set the specific integer code in a variable) allows this, but switching the

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-28 Thread Jeff King
On Tue, Apr 28, 2015 at 01:42:13PM -0700, Jonathan Nieder wrote: Jeff King wrote: But the NULL does not carry the information about _which_ error, and Erik is suggesting that the caller may need to change behavior based on that information. IOW, his current patch (return NULL and set the

Re: [PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-28 Thread Jeff King
On Sun, Apr 26, 2015 at 08:49:41AM +0200, Erik Elfström wrote: -extern const char *read_gitfile(const char *path); + +#define READ_GITFILE_ERR_STAT_FAILED 1 +#define READ_GITFILE_ERR_NOT_A_FILE 2 +#define READ_GITFILE_ERR_OPEN_FAILED 3 +#define READ_GITFILE_ERR_READ_FAILED 4 +#define

[PATCH v5 1/5] setup: add gentle version of read_gitfile

2015-04-26 Thread Erik Elfström
read_gitfile will die on most error cases. This makes it unsuitable for speculative calls. Extract the core logic and provide a gentle version that returns NULL on failure. The first usecase of the new gentle version will be to probe for submodules during git clean. Helped-by: Junio C Hamano