Re: [PATCH 56/67] avoid sprintf and strcpy with flex arrays

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Sep 21, 2015 at 01:11:09PM -0400, Eric Sunshine wrote:

> >> > -       p = xcalloc(1, sizeof(*p) + strlen(tmp_file) + 2);
> >> > -       strcpy(p->pack_name, tmp_file);
> >> > +       namelen = strlen(tmp_file) + 2;
> >>
> >> You mentioned this specially in the commit message, but from a brief
> >> read of the code, it's still not obvious (to me) why this is +2 rather
> >> than +1. Since you're touching the code anyhow, perhaps add an in-code
> >> comment explaining it?
> >
> > To be honest, I'm not sure what's going on with the "+ 2" here.
> >
> > In many cases with packed_git we allocate with "foo.idx" and want to be
> > able to later write "foo.pack" into the same buffer. But here we are
> > putting in a tmpfile name. This comes from 8455e48, but I don't see any
> > clue there. I wonder if the "+2" was simply cargo-culted from other
> > instances.
> 
> Ah, ok. I guess I misunderstood the commit message to mean or imply
> that the +2 was correct and sensible and well-understood.

I think it was more that I looked at other instances of packed_git, and
realized they could not be safely converted. I think "struct
alternate_object_database" has similar problems.

-Peff

PS As I mentioned earlier, I did end up adding a FLEX_ALLOC() macro in
   another series that builds on top of this. I haven't posted it yet,
   but check out:

     https://github.com/peff/git/commit/ba491c527572c763286b4b9519aef3c30482c2d1

   and

     https://github.com/peff/git/commit/d88444d5ba00bd875ef5291dca3b71dd046186dc

   if you are curious.
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]