Lars Schneider <larsxschneider@xxxxxxxxx> wrote:
> On 23 Jul 2016, at 10:14, Eric Wong <e@xxxxxxxxx> wrote:
> > larsxschneider@xxxxxxxxx wrote:
> >> +static struct cmd2process *start_protocol_filter(const char *cmd)
> >> +{
> >> + int ret = 1;
> >> + struct cmd2process *entry = NULL;
> >> + struct child_process *process = NULL;
> >
> > These are unconditionally set below, so initializing to NULL
> > may hide future bugs.
>
> OK. I thought it is generally a good thing to initialize a pointer with
> NULL. Can you explain to me how this might hide future bugs?
> I will remove the initialization.
Compilers complain about uninitialized variables. Blindly
setting them to NULL can allow them to be dereferenced;
triggering segfaults; especially if it's passed to a different
compilation unit the compiler can't see.
> >> +static int apply_protocol_filter(const char *path, const char *src, size_t len,
> >> + int fd, struct strbuf *dst, const char *cmd,
> >> + const char *filter_type)
> >> +{
<snip>
> >> + if (fd >= 0 && !src) {
> >> + ret &= fstat(fd, &fileStat) != -1;
> >> + len = fileStat.st_size;
> >
> > There's a truncation bug when sizeof(size_t) < sizeof(off_t)
>
> OK. What would you suggest to do in that case? Should we just let the
> filter fail? Is there anything else we could do?
Anything which refers to something on disk (or will eventually
stored there, such as blobs) should evolve towards off_t rather
than size_t. We just discovered a bunch of 32-bit truncation
bugs the other week:
https://public-inbox.org/git/1466807902.28869.8.camel@xxxxxxxxx/
If the protocol/ABI is frozen, it should probably fail;
and a 64-bit-off_t version for 32-bit systems should be defined.
--
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