Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

From: Stefan Beller <sbeller@google.com>
Date: Jul 1, 2016, 17:24 UTC
Message-ID: <CAGZ79kZxqRs=c-Fxe=uyCiPAw_R79gVtpyMn0OYc3OFrdev-hQ@mail.gmail.com>
In-Reply-To: <xmqqh9c9z3kf.fsf@gitster.mtv.corp.google.com>

On Fri, Jul 1, 2016 at 10:11 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> +static const char *stream_push_options_to_file()
>> +{
>> +     static const char *fname = "push-options-XXXXXX";
>> +     char *ret = xmallocz(strlen(fname));
>> +     int fd;
>> +     memcpy(ret, fname, strlen(fname));
>> +     fd = mkstemp(ret);
>
> Probably
>
>         char *ret;
>         int fd;
>
>         ret = xstrdup("push-options-XXXXXX");
>         fd = xmkstemp(ret);
>

That is way better to read! Also I did not know we also have a wrapper for mkstemp; Maybe I should check for any new syscall if we wrap that.

Show 36 quoted lines

> or use mkstemp but check the return value and "goto fail".
>
>> +     for (;;) {
>> +             char *line;
>> +             int len;
>> +
>> +             line = packet_read_line(0, &len);
>> +
>> +             if (!line)
>> +                     break;
>> +
>> +             if (write_in_full(fd, line, len) < 0 ||
>> +                 write_in_full(fd, "\n", 1) < 0)
>> +                     goto fail;
>> +     }
>> +
>> +     return ret;
>> +fail:
>> +     close(fd);
>> +     free(ret);
>> +     return NULL;
>> +}
>> +
>>  static const char *parse_pack_header(struct pack_header *hdr)
>>  {
>>       switch (read_pack_header(0, hdr)) {
>> @@ -1767,6 +1808,9 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
>>               const char *unpack_status = NULL;
>>               const char *push_options_file = NULL;
>>
>> +             if (use_push_options)
>> +                     push_options_file = stream_push_options_to_file();
>> +
>>               prepare_shallow_info(&si, &shallow);
>>               if (!si.nr_ours && !si.nr_theirs)
>>                       shallow_update = 0;