Skip to content

banned.h: fix vsprintf warning #322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

andportnoy
Copy link

Previously sprintf was the argument to the BANNED macro, where vsprintf
is expected.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 25, 2019

Welcome to GitGitGadget

Hi @andportnoy, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that this Pull Request has a good description, as it will be used as cover letter.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "commit:", and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a PR comment of the form /allow <username>.

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions.

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox ("raw") file corresponding to the mail you want to reply to from the Git mailing list. If you use GMail, you can upload that raw mbox file via:

curl -g --user "<EMailAddress>:<Password>" --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

Previously sprintf was the argument to the BANNED macro, where vsprintf
is expected.

Signed-off-by: Andrey Portnoy <[email protected]>
@andportnoy andportnoy force-pushed the fix-vsprintf-warning branch from bbf483a to 148e4c1 Compare August 25, 2019 18:18
@andportnoy
Copy link
Author

Force-pushed to sign off as advised by the PR bot.

@andportnoy
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 25, 2019

Error: User andportnoy is not permitted to use GitGitGadget

@dscho
Copy link
Member

dscho commented Aug 26, 2019

/allow andportnoy

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

User andportnoy is now allowed to use GitGitGadget.

@andportnoy
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

Submitted as [email protected]

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Andrey Portnoy via GitGitGadget" <[email protected]> writes:

> Previously sprintf was the argument to the BANNED macro, where vsprintf is
> expected.

Good eyes.  Thanks.


>
> Andrey Portnoy (1):
>   banned.h: fix vsprintf warning
>
>  banned.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>
> base-commit: 75b2f01a0f642b39b0f29b6218515df9b5eb798e
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-322%2Fandportnoy%2Ffix-vsprintf-warning-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-322/andportnoy/fix-vsprintf-warning-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/322

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

On the Git mailing list, Jeff King wrote (reply to this):

On Mon, Aug 26, 2019 at 09:24:10AM -0700, Junio C Hamano wrote:

> "Andrey Portnoy via GitGitGadget" <[email protected]> writes:
> 
> > Previously sprintf was the argument to the BANNED macro, where vsprintf is
> > expected.
> 
> Good eyes.  Thanks.

There's an identical patch in:

  https://public-inbox.org/git/cab687db8315dd4245e1703402a8c76218ee1115.1566762128.git.me@ttaylorr.com/

and both were inspired by:

  https://news.ycombinator.com/item?id=20793298

whose author has little info there, but I think is separate from either
submitter.

I don't know if we want to try to spread credit around via trailers.
"Racily-implemented-by:" ? :)

Anyway, the original bug is mine and the patch is obviously correct.

-Peff

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

On the Git mailing list, Taylor Blau wrote (reply to this):

On Mon, Aug 26, 2019 at 02:33:17PM -0400, Jeff King wrote:
> On Mon, Aug 26, 2019 at 09:24:10AM -0700, Junio C Hamano wrote:
>
> > "Andrey Portnoy via GitGitGadget" <[email protected]> writes:
> >
> > > Previously sprintf was the argument to the BANNED macro, where vsprintf is
> > > expected.
> >
> > Good eyes.  Thanks.
>
> There's an identical patch in:
>
>   https://public-inbox.org/git/cab687db8315dd4245e1703402a8c76218ee1115.1566762128.git.me@ttaylorr.com/

Thanks for mentioning. I did send mine in around a day ago now, but it
came in on a Sunday, so it makes sense that Andrey's message may have
appeared earlier in mailboxes when looking today.

I don't really mind about the credit, nor do I think there's much value
in crediting you or me with 'Racily-implemented-by'. My patch has a few
more details such as blame information (and how the typo was only made
in the explicit version, not the variadic form), so it may be worthwhile
to take that instead of this, but I don't mind either way.

> and both were inspired by:
>
>   https://news.ycombinator.com/item?id=20793298

Indeed. It was a little surprising to see 'banned.h' at the top of
Hacker News when I checked it this weekend :-).

> whose author has little info there, but I think is separate from either
> submitter.
>
> I don't know if we want to try to spread credit around via trailers.
> "Racily-implemented-by:" ? :)
>
> Anyway, the original bug is mine and the patch is obviously correct.
>
> -Peff

Yep.

Thanks,
Taylor

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

On the Git mailing list, Andrey Portnoy wrote (reply to this):



> On Aug 26, 2019, at 11:33 AM, Jeff King <[email protected]> wrote:
>=20
> On Mon, Aug 26, 2019 at 09:24:10AM -0700, Junio C Hamano wrote:
>=20
>> "Andrey Portnoy via GitGitGadget" <[email protected]> writes:
>>=20
>>> Previously sprintf was the argument to the BANNED macro, where =
vsprintf is
>>> expected.
>>=20
>> Good eyes.  Thanks.
>=20
> There's an identical patch in:
>=20
>  =
https://public-inbox.org/git/cab687db8315dd4245e1703402a8c76218ee1115.1566=
[email protected]/
>=20
> and both were inspired by:
>=20
>  https://news.ycombinator.com/item?id=3D20793298
Nope, mine was not =E2=80=9Cinspired by=E2=80=9D that comment, noticed =
the bug myself. I did look at this header because it was posted on HN, =
though.
>=20
> whose author has little info there, but I think is separate from =
either
> submitter.
>=20
> I don't know if we want to try to spread credit around via trailers.
> "Racily-implemented-by:" ? :)
>=20
> Anyway, the original bug is mine and the patch is obviously correct.
>=20
> -Peff
>=20

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

On the Git mailing list, Andrey Portnoy wrote (reply to this):



> On Aug 26, 2019, at 11:33 AM, Jeff King <[email protected]> wrote:
>=20
> On Mon, Aug 26, 2019 at 09:24:10AM -0700, Junio C Hamano wrote:
>=20
>> "Andrey Portnoy via GitGitGadget" <[email protected]> writes:
>>=20
>>> Previously sprintf was the argument to the BANNED macro, where =
vsprintf is
>>> expected.
>>=20
>> Good eyes.  Thanks.
>=20
> There's an identical patch in:
>=20
> =
https://public-inbox.org/git/cab687db8315dd4245e1703402a8c76218ee1115.1566=
[email protected]/
>=20
> and both were inspired by:
>=20
> https://news.ycombinator.com/item?id=3D20793298
Nope, mine was not =E2=80=9Cinspired by=E2=80=9D that comment, noticed =
the bug myself. I did look at this header because it was posted on HN, =
though.
>=20
> whose author has little info there, but I think is separate from =
either
> submitter.
>=20
> I don't know if we want to try to spread credit around via trailers.
> "Racily-implemented-by:" ? :)
>=20
> Anyway, the original bug is mine and the patch is obviously correct.
>=20
> -Peff
>=20

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

This branch is now known as tb/banned-vsprintf-namefix.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

This patch series was integrated into pu via git@9e6bbff.

@gitgitgadget gitgitgadget bot added the pu label Aug 26, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Aug 27, 2019

On the Git mailing list, Junio C Hamano wrote (reply to this):

Taylor Blau <[email protected]> writes:

> ... My patch has a few
> more details such as blame information (and how the typo was only made
> in the explicit version, not the variadic form), so it may be worthwhile
> to take that instead of this, but I don't mind either way.

Yup, that was exactly why I took that version (after tentatively
queuing the other one, but discarded before pushing the integration
result out).

Thanks, all.

>> I don't know if we want to try to spread credit around via trailers.
>> "Racily-implemented-by:" ? :)

Please don't X-<.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 3, 2019

This patch series was integrated into pu via git@f7f40e4.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 5, 2019

This patch series was integrated into pu via git@d04406d.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 7, 2019

This patch series was integrated into pu via git@12f12fc.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 9, 2019

This patch series was integrated into pu via git@024bf27.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 9, 2019

This patch series was integrated into next via git@e0f6d18.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 16, 2019

This patch series was integrated into pu via git@bb5d4ee.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 17, 2019

This patch series was integrated into pu via git@f0ace75.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 18, 2019

This patch series was integrated into pu via git@1e208eb.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 18, 2019

This patch series was integrated into pu via git@be13edb.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

This patch series was integrated into pu via git@37801f0.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

This patch series was integrated into master via git@37801f0.

@gitgitgadget gitgitgadget bot added the master label Sep 30, 2019
@gitgitgadget gitgitgadget bot closed this Sep 30, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

Closed via 37801f0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants