All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Marat Radchenko <marat@slonopotamus.org>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v4 0/3] Take four on fixing OPT_SET_PTR issues
Date: Mon, 31 Mar 2014 10:23:44 -0700	[thread overview]
Message-ID: <xmqq4n2e8eov.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <cover.1396177207.git.marat@slonopotamus.org> (Marat Radchenko's message of "Sun, 30 Mar 2014 15:08:20 +0400")

Marat Radchenko <marat@slonopotamus.org> writes:

> Patches summary:
> 1. Fix initial issue (incorrect cast causing crash on 64-bit MSVC)
> 2. Improve OPT_SET_PTR to prevent same errors in future
> 3. Purge OPT_SET_PTR away since nobody uses it
>
> *Optional* patch №3 is separated from №1 and №2 so that if someone someday
> decides to return OPT_SET_PTR back by reverting №3, it will be returned
> in a sane state.
>
> Decision of (not) merging №3 is left as an exercise to the reader due to
> my insufficient knowledge of accepted practices in Git project.

SET_PTR() may not be used, but are there places where SET_INT() is
abused with a cast-to-pointer for the same effect?  I didn't check,
but if there are such places, converting them to use SET_PTR() with
their existing cast removed may be a better way to go.

My suspicion is that there would be none, as switching the behaviour
based on a small integer flag value is far easier than swapping the
pointer to a pointee to be operated on, when responding to a command
line option.

  parent reply	other threads:[~2014-03-31 17:23 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-28 12:04 [PATCH] MSVC: fix t0040-parse-options Marat Radchenko
2014-03-28 18:19 ` Junio C Hamano
2014-03-29 19:59   ` [PATCH v2] MSVC: fix t0040-parse-options crash Marat Radchenko
2014-03-29 20:09 ` [PATCH v3] " Marat Radchenko
2014-03-29 21:34   ` Andreas Schwab
2014-03-29 22:17     ` René Scharfe
2014-03-30  2:01   ` Junio C Hamano
2014-03-30  8:29     ` Andreas Schwab
2014-03-31 21:09       ` Jeff King
2014-03-30 11:08     ` [PATCH v4 0/3] Take four on fixing OPT_SET_PTR issues Marat Radchenko
2014-03-30 11:08       ` [PATCH v4 1/3] MSVC: fix t0040-parse-options crash Marat Radchenko
2014-03-30 11:08       ` [PATCH v4 2/3] parse-options: add cast to correct pointer type to OPT_SET_PTR Marat Radchenko
2014-03-31 17:16         ` Junio C Hamano
2014-03-30 11:08       ` [PATCH v4 3/3] parse-options: remove unused OPT_SET_PTR Marat Radchenko
2014-03-31 17:23       ` Junio C Hamano [this message]
2014-03-31 21:07         ` [PATCH v4 0/3] Take four on fixing OPT_SET_PTR issues Jeff King
2014-03-31 22:54           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq4n2e8eov.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=marat@slonopotamus.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.