Re: [PATCH v2 2/3] textconv: support for blame

[resending to cc git@vger]

On Tue, Jun 15, 2010 at 12:29:43PM +0200, bonneta wrote:

We have changed the declaration of textconv_object() to:

static int textconv_object(const char *path,
                           const unsigned char *sha1,
                           char **buf,
                           unsigned long *buf_size)

And now we can do:
*buf_size = fill_textconv(textconv, df, buf);
without any cast.

I assume you mean dropping the final buf_size parameter from that
declaration, which is what your usage example has. I would return either
an "unsigned long" or a size_t rather than an int. We are dealing with
potential whole-file sizes, so it is better to use at least as large a
data type as other parts of the code (we still may run into truncation
problems, but at least you are not making things any worse).

But we have to do:
textconv_object(read_from, null_sha1, &buf.buf, (unsigned long *)
&buf.len))
where buf.len is size_t.

Is that ok?

No, that has the same problem. Imagine a big endian machine with a
32-bit unsigned long and a 64-bit size_t. You would write into the first
32 bits of buf.len, which are the high bits, giving you a ridiculously
large answer.

The only portable way in C to convert between types is by assignment. So
you have to do:

  unsigned long foo;
  textconv_object(read_from, null_sha1, &buf.buf, &foo);
  buf.len = foo;

But now I'm confused. That matches the declaration you gave in the first
part of your email, but not the usage example.

-Peff