* Re: Re: [PATCH v3 4/9] user-manual: Mention 'git remote add' for remote branch config
From: W. Trevor King @ 2013-02-18 2:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git, Jonathan Nieder
In-Reply-To: <7vwqu6cq7i.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 695 bytes --]
On Sun, Feb 17, 2013 at 06:26:25PM -0800, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> > -------------------------------------------------
> > +$ git fetch git://example.com/proj.git +refs/heads/*:refs/remotes/example/*
> > +$ git fetch example +refs/heads/*:refs/remotes/example/*
> > $ git fetch example
> > -------------------------------------------------
>
> These _three_ commands will do the same.
Oops. Feel free to fix this for me, and consider the revised version
signed off by me ;).
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 6/9] user-manual: Use 'git config --global user.*' for setup
From: Junio C Hamano @ 2013-02-18 2:47 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git, Jonathan Nieder
In-Reply-To: <27886ef1313956bc19ddb9e658c4eb181fab2e46.1361146398.git.wking@tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
> From: "W. Trevor King" <wking@tremily.us>
>
> A simple command line call is easier than spawning an editor,
> especially for folks new to ideas like the "command line" and "text
> editors". This is also the approach suggested by 'git commit' if you
> try and commit without having configured user.name or user.email.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> Documentation/user-manual.txt | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
This looks like a good 'maint' material that can be applied straight
away there in preparation for 1.8.1.4 to me (modulo typo); reviewers
watching from the sideline, please stop me if you see issues.
>
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index a79b3e31..d9276d7 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -991,9 +991,16 @@ Developing with Git
> Telling Git your name
> ---------------------
>
> -Before creating any commits, you should introduce yourself to Git. The
> -easiest way to do so is to make sure the following lines appear in a
> -file named `.gitconfig` in your home directory:
> +Before creating any commits, you should introduce yourself to Git.
> +The easiest way to do so is to use linkgit:git-config[1]:
> +
> +------------------------------------------------
> +$ git config --global user.name 'Your Name Comes Here'
> +$ git config --global user.email 'you@yourdomain.example.com'
> +------------------------------------------------
> +
> +Which will adds the following to a file named `.gitconfig` in your
s/adds/add/;
> +home directory:
>
> ------------------------------------------------
> [user]
> @@ -1001,8 +1008,9 @@ file named `.gitconfig` in your home directory:
> email = you@yourdomain.example.com
> ------------------------------------------------
>
> -(See the "CONFIGURATION FILE" section of linkgit:git-config[1] for
> -details on the configuration file.)
> +See the "CONFIGURATION FILE" section of linkgit:git-config[1] for
> +details on the configuration file. The file is plain text, so you can
> +also edit it with your favorite editor.
>
>
> [[creating-a-new-repository]]
^ permalink raw reply
* Re: [PATCH v3 7/9] user-manual: Use request-pull to generate "please pull" text
From: Junio C Hamano @ 2013-02-18 2:50 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git, Jonathan Nieder
In-Reply-To: <edd0062a7f54300cccd0123dc108e9a0f39bdccc.1361146398.git.wking@tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
> From: "W. Trevor King" <wking@tremily.us>
>
> Less work and more error checking (e.g. does a merge base exist?).
> Add an explicit push before request-pull to satisfy request-pull,
> which checks to make sure the references are publically available.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> Documentation/user-manual.txt | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index d9276d7..cf09ddf 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -2304,16 +2304,11 @@ these changes, just apply directly to the `release` branch, and then
> merge that into the `test` branch.
>
> To create diffstat and shortlog summaries of changes to include in a "please
> -pull" request to Linus you can use:
> +pull" request to Linus you can use linkgit:git-request-pull[1]:
The command you refer to does the _whole_ thing, not just to create
diffstat and shortlog summaries to be included.
> -------------------------------------------------
> +$ git push mytree
> +$ git request-pull origin mytree release
> -------------------------------------------------
The introductory text to tell the reader what they are being taught
needs to be updated a bit more, I think.
After pushing your work to "mytree", you can use request-pull
command to prepare a "please pull" request message to send
to Linus, like so:
or something.
>
> Here are some of the scripts that simplify all this even further.
^ permalink raw reply
* Re: [PATCH v3 8/9] user-manual: Flesh out uncommitted changes and submodule updates
From: Junio C Hamano @ 2013-02-18 2:53 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git, Jonathan Nieder
In-Reply-To: <6219cb47a0a5b7428ae6c729e9a1a55344f47428.1361146398.git.wking@tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
> From: "W. Trevor King" <wking@tremily.us>
>
> If you try and update a submodule with a dirty working directory, you
> get an error message like:
>
> $ git submodule update
> error: Your local changes to the following files would be overwritten by checkout:
> ...
> Please, commit your changes or stash them before you can switch branches.
> Aborting
> ...
>
> Mention this in the submodule notes. The previous phrase was short
> enough that I originally thought it might have been referring to the
> reflog note (obviously, uncommitted changes will not show up in the
> reflog either ;).
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> Documentation/user-manual.txt | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index cf09ddf..3381c22 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -3729,7 +3729,9 @@ module a
>
> NOTE: The changes are still visible in the submodule's reflog.
>
> +If you did not commit your submodule changes, the changes will *not*
> +be silently overwritten. Instead, you get the usual warning about not
> +being able switch from a dirty branch.
The scenario this talks about is to commit changes in the
superproject and then to run "submodule update". I think the above
clarification is still incomplete. You may have committed in the
submodule some changes but not all.
If you have uncommitted changes in your submodule working
tree, "git submodule update" will not overwrite them.
Instead...
would be an improvement, I think.
>
> [[low-level-operations]]
> Low-level Git operations
^ permalink raw reply
* Re: [PATCH v3 9/9] user-manual: Use -o latest.tar.gz to create a gzipped tarball
From: Junio C Hamano @ 2013-02-18 2:58 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git, Jonathan Nieder
In-Reply-To: <27dea793fc101f8bd148385b753997867b987658.1361146398.git.wking@tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
> From: "W. Trevor King" <wking@tremily.us>
>
> This functionality was introduced by 0e804e09 (archive: provide
> builtin .tar.gz filter, 2011-07-21) for v1.7.7.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
This looks like a good 'maint' material that can be applied straight
away there if the patch did not depend on 5/9 to cause needless
conflicts.
It is easy for me to deal with conflicts in this particular case,
but in general it would have been more straightforward to manage if
these more localized phrasing fixes came earlier and a larger
file-wide cosmetic change was done after all the others have
settled.
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index 3381c22..af6c09d 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -931,11 +931,20 @@ The linkgit:git-archive[1] command can create a tar or zip archive from
> any version of a project; for example:
>
> -------------------------------------------------
> -$ git archive --format=tar --prefix=project/ HEAD | gzip >latest.tar.gz
> +$ git archive -o latest.tar.gz --prefix=project/ HEAD
> -------------------------------------------------
>
> -will use HEAD to produce a tar archive in which each filename is
> -preceded by `project/`.
> +will use HEAD to produce a gzipped tar archive in which each filename
> +is preceded by `project/`. The output file format is inferred from
> +the output file extension if possible, see linkgit:git-archive[1] for
> +details.
> +
> +Versions of Git older than 1.7.7 don't know about the 'tar.gz' format,
> +you'll need to use gzip explicitly:
> +
> +-------------------------------------------------
> +$ git archive --format=tar --prefix=project/ HEAD | gzip >latest.tar.gz
> +-------------------------------------------------
>
> If you're releasing a new version of a software project, you may want
> to simultaneously make a changelog to include in the release
^ permalink raw reply
* Re: [PATCH v3 2/9] user-manual: Reorganize the reroll sections, adding 'git rebase -i'
From: W. Trevor King @ 2013-02-18 2:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git, Jonathan Nieder
In-Reply-To: <7v621qe4xc.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]
On Sun, Feb 17, 2013 at 06:23:11PM -0800, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> > [[rewriting-one-commit]]
> > Rewriting a single commit
> > -------------------------
> > @@ -2552,71 +2558,81 @@ $ git commit --amend
> > which will replace the old commit by a new commit incorporating your
> > changes, giving you a chance to edit the old commit message first.
> >
>
> ... A lot of lines removed here ...
>
> > +[[reordering-patch-series]]
>
> This change makes the [[rewriting-one-commit]] section say "We
> already saw you can do 'commit --amend'" and nothing else. It makes
> me wonder if the remaining section is worth keeping if we go this
> route.
I think there should be some mention of `commit --amend` in the
rewriting history section. It's a pretty straightforward idea though,
so there's not all that much to say about it ;).
> > +Then modify, reorder, or eliminate patches as preferred before applying
> > +them again with linkgit:git-am[1]:
> >
> > -------------------------------------------------
> > +$ git am *.patch
> > -------------------------------------------------
>
> It may be just me, but s/preferred/needed/, perhaps?
Sure. I was just shifting the existing text. Will fix in v4.
> > +The rebase will stop when `pick` has been replaced with `edit` or when
> > +a command fails due to merge errors. When you are done editing and/or
> > +resolving conflicts...
>
> I am afraid that "due to merge errors" and "resolving conflicts" do
> not look corresponding to each other for a new reader. Also here we
> say "when a command fails", but the explanation before this part
> never says "list of commands".
Oops. s/command/commit/.
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/9] user-manual: Reorganize the reroll sections, adding 'git rebase -i'
From: Junio C Hamano @ 2013-02-18 3:48 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git, Jonathan Nieder
In-Reply-To: <20130218023908.GC5936@odin.tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
> On Sun, Feb 17, 2013 at 06:23:11PM -0800, Junio C Hamano wrote:
>> "W. Trevor King" <wking@tremily.us> writes:
>> > [[rewriting-one-commit]]
>> > Rewriting a single commit
>> > -------------------------
>> > @@ -2552,71 +2558,81 @@ $ git commit --amend
>> > which will replace the old commit by a new commit incorporating your
>> > changes, giving you a chance to edit the old commit message first.
>> >
>>
>> ... A lot of lines removed here ...
>>
>> > +[[reordering-patch-series]]
>>
>> This change makes the [[rewriting-one-commit]] section say "We
>> already saw you can do 'commit --amend'" and nothing else. It makes
>> me wonder if the remaining section is worth keeping if we go this
>> route.
>
> I think there should be some mention of `commit --amend` in the
> rewriting history section. It's a pretty straightforward idea though,
> so there's not all that much to say about it ;).
I do think teaching "commit --amend", "rebase" and "rebase -i"
together makes sense. It just felt that the remaining piece on
"commit --amend" looked too thin in the substance, but it may be a
good change in the existing flow of text.
^ permalink raw reply
* Please pull l10n updates for 1.8.2 round 2
From: Jiang Xin @ 2013-02-18 6:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git List, Byrial Jensen, Ralf Thielow,
Ævar Arnfjörð Bjarmason, Marco Paolone,
Vincent van Ravesteijn, Marco Sousa, Peter Krefting,
Trần Ngọc Quân, David Hrbáč
Hi, Junio
Please pull l10n updates for git 1.8.2 round 2.
BTW, I find there are new updates in master branch and next branch:
$ po-helper.sh check update
New l10n updates found in "master" branch of remote "<kernel>":
5 new messages.
New l10n updates found in "next" branch of remote "<kernel>":
6 new, 2 removed messages.
So there will be another 1 or 2 rounds of translations.
The following changes since commit 5bf72ed2e7ef82d5ef7a35c978dc72576fdb0efc:
Merge branch 'maint' (2013-02-12 12:23:12 -0800)
are available in the git repository at:
git://github.com/git-l10n/git-po master
for you to fetch changes up to a77c07d9747fdba7aef22374a91d4b6aaf2f7b2f:
l10n: zh_CN.po: translate 35 new messages (2013-02-18 09:52:33 +0800)
----------------------------------------------------------------
Jiang Xin (2):
l10n: Update git.pot (35 new, 14 removed messages)
l10n: zh_CN.po: translate 35 new messages
Tran Ngoc Quan (1):
l10n: vi.po: update new strings (2004t0u0f)
po/git.pot | 1323 +++++++++++++++++++++++++++++-------------------------
po/vi.po | 1442 ++++++++++++++++++++++++++++++++---------------------------
po/zh_CN.po | 1392 ++++++++++++++++++++++++++++++--------------------------
3 files changed, 2284 insertions(+), 1873 deletions(-)
^ permalink raw reply
* Re: git clone tag shallow
From: Duy Nguyen @ 2013-02-18 6:49 UTC (permalink / raw)
To: Thibault Kruse; +Cc: git
In-Reply-To: <CAByu6UWEF48XvTmRnVDb2Bqxy1dNdUSXpTuy804215Vgs_KJxw@mail.gmail.com>
On Mon, Feb 18, 2013 at 2:13 AM, Thibault Kruse
<tibokruse@googlemail.com> wrote:
> Hi all,
>
> I notice that using git 1.8.3, I can call
> git clone repo1 repo2 --branch tagname
> with a tag, not a branch. Is this going to be a stable and documented feature?
There is a test for --branch=tag in t5601, so I say it's supported. If
the document is not clear, patches are welcome.
--
Duy
^ permalink raw reply
* Re: Please pull l10n updates for 1.8.2 round 2
From: Junio C Hamano @ 2013-02-18 8:02 UTC (permalink / raw)
To: Jiang Xin
Cc: Git List, Byrial Jensen, Ralf Thielow,
Ævar Arnfjörð Bjarmason, Marco Paolone,
Vincent van Ravesteijn, Marco Sousa, Peter Krefting,
Trần Ngọc Quân, David Hrbáč
In-Reply-To: <CANYiYbHRzS6EUgRBekc+m0Sf3nYDcb9108xk1goH+=QDw36EDA@mail.gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> Please pull l10n updates for git 1.8.2 round 2.
>
> BTW, I find there are new updates in master branch and next branch:
>
> $ po-helper.sh check update
> New l10n updates found in "master" branch of remote "<kernel>":
> 5 new messages.
>
> New l10n updates found in "next" branch of remote "<kernel>":
> 6 new, 2 removed messages.
>
> So there will be another 1 or 2 rounds of translations.
Thanks.
^ permalink raw reply
* Re: git clone tag shallow
From: Thibault Kruse @ 2013-02-18 8:26 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git
In-Reply-To: <CACsJy8Dso-g7foyJhpY20DNrY11PA8ZZUmP6JXxsiJ_Ggbt_KA@mail.gmail.com>
Hi Duy,
Looking closer at the rest of the docs, I guess it is consistent, but
not terribly helpful.
Throughout the docs (--help and man), it is never very clear what
objects a command
may accept or not. here is what I get from it:
Whenever a command description involves "<branch>" this can, depending
on the command, refer to
1) a name that, when prepended with "refs/heads/", is a valid ref,
2) a name that, when prepended with "refs/heads/" or "refs/tags", is a
valid ref,
3) a name that, when prepended with "refs/[heads|tags]/", or unique in
"refs/remotes/*/" is a valid ref
Now in the docu I don't see a nice distinction between 1), 2) and 3).
I could work on a patch if someone
tells me how to clearly distinguish those cases.
Example for 1)
git clone --branch <branch> in git 1.7.x
Example for 2)
git clone --branch <branch> in git 1.8.x
Example for 3)
git checkout <branch> in git 1.8.x
I would like the docs to relate to those e.g. as
1) git clone --branch <local-branch>
2) git clone --branch <local-branch-or-tag>
3) git checkout <commit-label>
or something similar, in the future. But I wont create a patch before
having some blessing on the labels to use.
On a related note, what would help my use-case would be an extension
to git clone such
that git clone [--deep] accepted also most other kinds of commit
references, such as SHA-IDs.
Such that
git clone --deep --branch 2h134vjvbc
would work. Is there a plan to introduce that as well?
cheers,
Thibault
On Mon, Feb 18, 2013 at 7:49 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Mon, Feb 18, 2013 at 2:13 AM, Thibault Kruse
> <tibokruse@googlemail.com> wrote:
>> Hi all,
>>
>> I notice that using git 1.8.3, I can call
>> git clone repo1 repo2 --branch tagname
>> with a tag, not a branch. Is this going to be a stable and documented feature?
>
> There is a test for --branch=tag in t5601, so I say it's supported. If
> the document is not clear, patches are welcome.
> --
> Duy
^ permalink raw reply
* Re: [PATCH v3 0/9] User manual updates
From: Junio C Hamano @ 2013-02-18 8:56 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git, Jonathan Nieder
In-Reply-To: <cover.1361146398.git.wking@tremily.us>
I've taken the following to 'maint', some with minor and obvious
fix-ups:
user-manual: use 'remote add' to setup push URLs
user-manual: give 'git push -f' as an alternative to +master
user-manual: mention 'git remote add' for remote branch config
user-manual: use 'git config --global user.*' for setup
user-manual: use -o latest.tar.gz to create a gzipped tarball
I think I've sent reviews for all others, except for the `backtick`
one. I think it is more efficient to do that one _after_ we are
done with all the others.
Thanks.
^ permalink raw reply
* Re: [PATCH 2/3] remote-curl: verify smart-http metadata lines
From: Jeff King @ 2013-02-18 8:59 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218005443.GB3221@elie.Belkin>
On Sun, Feb 17, 2013 at 04:54:43PM -0800, Jonathan Nieder wrote:
> > My intent was that it followed the error convention of "negative is
> > error, 0 is success, and positive is not used, but reserved for
> > future use".
>
> From a maintainability perspective, that kind of contract would be
> dangerous, since some *other* caller could arrive and use the function
> without a "< 0" without knowing it is doing anything wrong. When new
> return values appear, the function should be renamed to help the patch
> author and reviewers remember to check all callers.
True. That's why I always write "< 0". :)
> That is, from the point of view of maintainability, there is no
> distinction between "if (read_packets_until_... < 0)" and
> "if (read_packets_until_...)" and either form is fine.
>
> My comment was just to say the "< 0" forced me to pause a moment and
> check out the implementation. This is basically a stylistic thing and
> if you prefer to keep the "< 0", that's fine with me.
Interesting. To me, "foo() < 0" just reads idiomatically as "error-check
the foo call".
Anyway, I've redone the patch series to just re-use get_remote_heads,
which is more robust. So this function has gone away in the new version.
-Peff
^ permalink raw reply
* [PATCHv2 0/10] pkt-line and remote-curl cleanups server
From: Jeff King @ 2013-02-18 9:12 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218014113.GC3221@elie.Belkin>
On Sun, Feb 17, 2013 at 05:41:13PM -0800, Jonathan Nieder wrote:
> > I don't think so. Don't ERR lines appear inside their own packets?
>
> Yes, I misread get_remote_heads for some reason. Thanks for checking.
Thanks for bringing it up. I had not even thought about ERR at all. So
it was luck rather than skill that I was right. :)
> I'm not sure whether servers are expected to send a flush after an
> ERR packet. The only codepath I know of in git itself that sends
> such packets is git-daemon, which does not flush after the error (but
> is not used in the stateless-rpc case). http-backend uses HTTP error
> codes for its errors.
I just checked, and GitHub also does not send flush packets after ERR.
Which makes sense; ERR is supposed to end the conversation. I can change
GitHub, of course, but who knows what other implementations exist (e.g.,
I do not know off-hand whether gitolite has custom ERR responses). So it
seems pretty clear that just checking for a flush packet is not the
right thing, and we need to actually parse the packet contents (at least
to some degree).
> If I am reading get_remote_heads correctly, calling it with the
> following tweak should work ok. The extra thread is just to feed a
> string into a fd-based interface and could be avoided for "list", too,
> if it costs too much.
Yeah, your patch does work, though we miss out on some of the refname
checks. I think what I'd rather do is just teach get_remote_heads to
read from a buffer (to avoid the extra thread and pipe), and then just
run (and cache) the ref parsing unconditionally once we've read from the
server. It shouldn't make a difference in the normal case, as we would
usually do a "list" anyway (and by caching, "list" can just feed out the
cached copy).
While looking into this, I noticed a bunch of other possible cleanups.
Patches to follow:
[01/10]: pkt-line: move a misplaced comment
[02/10]: pkt-line: drop safe_write function
[03/10]: pkt-line: clean up "gentle" reading function
[04/10]: pkt-line: change error message for oversized packet
[05/10]: pkt-line: rename s/packet_read_line/packet_read/
These are all just cleanups I noticed while looking at pkt-line. Any of
them can be dropped, though there would be some textual conflicts for
the later patches.
[06/10]: pkt-line: share buffer/descriptor reading implementation
[07/10]: teach get_remote_heads to read from a memory buffer
[08/10]: remote-curl: pass buffer straight to get_remote_heads
These all build on each other to get rid of the extra thread/pipe, which
I think is worth doing even without the rest of the series.
[09/10]: remote-curl: move ref-parsing code up in file
[10/10]: remote-curl: always parse incoming refs
And these ones actually fix the problem I noticed.
-Peff
^ permalink raw reply
* [PATCHv2 01/10] pkt-line: move a misplaced comment
From: Jeff King @ 2013-02-18 9:14 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
The comment describing the packet writing interface was
originally written above packet_write, but migrated to be
above safe_write in f3a3214, probably because it is meant to
generally describe the packet writing interface and not a
single function. Let's move it into the header file, where
users of the interface are more likely to see it.
Signed-off-by: Jeff King <peff@peff.net>
---
I just left the comment intact as I moved it. It kind of implies to me
that you hand a big buffer to these functions and they would packetize
it for you, which is not true. I don't know if anybody else sees that;
it might be worth tweaking the text.
pkt-line.c | 15 ---------------
pkt-line.h | 14 +++++++++++++-
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index eaba15f..5138f47 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -46,21 +46,6 @@ static void packet_trace(const char *buf, unsigned int len, int write)
strbuf_release(&out);
}
-/*
- * Write a packetized stream, where each line is preceded by
- * its length (including the header) as a 4-byte hex number.
- * A length of 'zero' means end of stream (and a length of 1-3
- * would be an error).
- *
- * This is all pretty stupid, but we use this packetized line
- * format to make a streaming format possible without ever
- * over-running the read buffers. That way we'll never read
- * into what might be the pack data (which should go to another
- * process entirely).
- *
- * The writing side could use stdio, but since the reading
- * side can't, we stay with pure read/write interfaces.
- */
ssize_t safe_write(int fd, const void *buf, ssize_t n)
{
ssize_t nn = n;
diff --git a/pkt-line.h b/pkt-line.h
index 8cfeb0c..7a67e9c 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -5,7 +5,19 @@
#include "strbuf.h"
/*
- * Silly packetized line writing interface
+ * Write a packetized stream, where each line is preceded by
+ * its length (including the header) as a 4-byte hex number.
+ * A length of 'zero' means end of stream (and a length of 1-3
+ * would be an error).
+ *
+ * This is all pretty stupid, but we use this packetized line
+ * format to make a streaming format possible without ever
+ * over-running the read buffers. That way we'll never read
+ * into what might be the pack data (which should go to another
+ * process entirely).
+ *
+ * The writing side could use stdio, but since the reading
+ * side can't, we stay with pure read/write interfaces.
*/
void packet_flush(int fd);
void packet_write(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
--
1.8.1.20.g7078b03
^ permalink raw reply related
* [PATCHv2 02/10] pkt-line: drop safe_write function
From: Jeff King @ 2013-02-18 9:15 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
This is just write_or_die by another name.
Signed-off-by: Jeff King <peff@peff.net>
---
Actually, they are not quite the same. write_or_die will exit(0) when it
sees EPIPE. Which makes me a little nervous.
builtin/receive-pack.c | 2 +-
builtin/send-pack.c | 2 +-
fetch-pack.c | 2 +-
http-backend.c | 8 ++++----
pkt-line.c | 21 ++-------------------
remote-curl.c | 4 ++--
send-pack.c | 2 +-
sideband.c | 9 +++++----
upload-pack.c | 3 ++-
9 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e8878de..48cd5dc 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -908,7 +908,7 @@ static void report(struct command *commands, const char *unpack_status)
if (use_sideband)
send_sideband(1, 1, buf.buf, buf.len, use_sideband);
else
- safe_write(1, buf.buf, buf.len);
+ write_or_die(1, buf.buf, buf.len);
strbuf_release(&buf);
}
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 57a46b2..8778519 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -79,7 +79,7 @@ static void print_helper_status(struct ref *ref)
}
strbuf_addch(&buf, '\n');
- safe_write(1, buf.buf, buf.len);
+ write_or_die(1, buf.buf, buf.len);
}
strbuf_release(&buf);
}
diff --git a/fetch-pack.c b/fetch-pack.c
index 6d8926a..b90dadf 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -245,7 +245,7 @@ static void send_request(struct fetch_pack_args *args,
send_sideband(fd, -1, buf->buf, buf->len, LARGE_PACKET_MAX);
packet_flush(fd);
} else
- safe_write(fd, buf->buf, buf->len);
+ write_or_die(fd, buf->buf, buf->len);
}
static void insert_one_alternate_ref(const struct ref *ref, void *unused)
diff --git a/http-backend.c b/http-backend.c
index f50e77f..8144f3a 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -70,7 +70,7 @@ static void format_write(int fd, const char *fmt, ...)
if (n >= sizeof(buffer))
die("protocol error: impossibly long line");
- safe_write(fd, buffer, n);
+ write_or_die(fd, buffer, n);
}
static void http_status(unsigned code, const char *msg)
@@ -111,7 +111,7 @@ static void end_headers(void)
static void end_headers(void)
{
- safe_write(1, "\r\n", 2);
+ write_or_die(1, "\r\n", 2);
}
__attribute__((format (printf, 1, 2)))
@@ -157,7 +157,7 @@ static void send_strbuf(const char *type, struct strbuf *buf)
hdr_int(content_length, buf->len);
hdr_str(content_type, type);
end_headers();
- safe_write(1, buf->buf, buf->len);
+ write_or_die(1, buf->buf, buf->len);
}
static void send_local_file(const char *the_type, const char *name)
@@ -185,7 +185,7 @@ static void send_local_file(const char *the_type, const char *name)
die_errno("Cannot read '%s'", p);
if (!n)
break;
- safe_write(1, buf, n);
+ write_or_die(1, buf, n);
}
close(fd);
free(buf);
diff --git a/pkt-line.c b/pkt-line.c
index 5138f47..699c2dd 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -46,23 +46,6 @@ static void packet_trace(const char *buf, unsigned int len, int write)
strbuf_release(&out);
}
-ssize_t safe_write(int fd, const void *buf, ssize_t n)
-{
- ssize_t nn = n;
- while (n) {
- int ret = xwrite(fd, buf, n);
- if (ret > 0) {
- buf = (char *) buf + ret;
- n -= ret;
- continue;
- }
- if (!ret)
- die("write error (disk full?)");
- die_errno("write error");
- }
- return nn;
-}
-
/*
* If we buffered things up above (we don't, but we should),
* we'd flush it here
@@ -70,7 +53,7 @@ void packet_flush(int fd)
void packet_flush(int fd)
{
packet_trace("0000", 4, 1);
- safe_write(fd, "0000", 4);
+ write_or_die(fd, "0000", 4);
}
void packet_buf_flush(struct strbuf *buf)
@@ -106,7 +89,7 @@ void packet_write(int fd, const char *fmt, ...)
va_start(args, fmt);
n = format_packet(fmt, args);
va_end(args);
- safe_write(fd, buffer, n);
+ write_or_die(fd, buffer, n);
}
void packet_buf_write(struct strbuf *buf, const char *fmt, ...)
diff --git a/remote-curl.c b/remote-curl.c
index 933c69a..7be4b53 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -685,7 +685,7 @@ static int fetch_git(struct discovery *heads,
err = rpc_service(&rpc, heads);
if (rpc.result.len)
- safe_write(1, rpc.result.buf, rpc.result.len);
+ write_or_die(1, rpc.result.buf, rpc.result.len);
strbuf_release(&rpc.result);
strbuf_release(&preamble);
free(depth_arg);
@@ -805,7 +805,7 @@ static int push_git(struct discovery *heads, int nr_spec, char **specs)
err = rpc_service(&rpc, heads);
if (rpc.result.len)
- safe_write(1, rpc.result.buf, rpc.result.len);
+ write_or_die(1, rpc.result.buf, rpc.result.len);
strbuf_release(&rpc.result);
free(argv);
return err;
diff --git a/send-pack.c b/send-pack.c
index 97ab336..a99a1e4 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -281,7 +281,7 @@ int send_pack(struct send_pack_args *args,
send_sideband(out, -1, req_buf.buf, req_buf.len, LARGE_PACKET_MAX);
}
} else {
- safe_write(out, req_buf.buf, req_buf.len);
+ write_or_die(out, req_buf.buf, req_buf.len);
packet_flush(out);
}
strbuf_release(&req_buf);
diff --git a/sideband.c b/sideband.c
index d5ffa1c..8f7b25b 100644
--- a/sideband.c
+++ b/sideband.c
@@ -1,3 +1,4 @@
+#include "cache.h"
#include "pkt-line.h"
#include "sideband.h"
@@ -108,7 +109,7 @@ int recv_sideband(const char *me, int in_stream, int out)
} while (len);
continue;
case 1:
- safe_write(out, buf + pf+1, len);
+ write_or_die(out, buf + pf+1, len);
continue;
default:
fprintf(stderr, "%s: protocol error: bad band #%d\n",
@@ -138,12 +139,12 @@ ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet
if (0 <= band) {
sprintf(hdr, "%04x", n + 5);
hdr[4] = band;
- safe_write(fd, hdr, 5);
+ write_or_die(fd, hdr, 5);
} else {
sprintf(hdr, "%04x", n + 4);
- safe_write(fd, hdr, 4);
+ write_or_die(fd, hdr, 4);
}
- safe_write(fd, p, n);
+ write_or_die(fd, p, n);
p += n;
sz -= n;
}
diff --git a/upload-pack.c b/upload-pack.c
index 7c05b15..04d6bd7 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -69,7 +69,8 @@ static ssize_t send_client_data(int fd, const char *data, ssize_t sz)
xwrite(fd, data, sz);
return sz;
}
- return safe_write(fd, data, sz);
+ write_or_die(fd, data, sz);
+ return sz;
}
static FILE *pack_pipe = NULL;
--
1.8.1.20.g7078b03
^ permalink raw reply related
* [PATCHv2 03/10] pkt-line: clean up "gentle" reading function
From: Jeff King @ 2013-02-18 9:16 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
Originally we had a single function for reading packetized
data: packet_read_line. Commit 46284dd grew a more "gentle"
form that would return an error instead of dying upon
reading a truncated input stream. However:
1. The two functions were called "packet_read" and
"packet_read_line", with no indication that the only
difference is in the error handling.
2. There was no documentation about which error conditions
were handled in the gentle form, and which still caused
a death.
3. The internal variable to trigger the gentle mode was
called "return_line_fail", which was not very
expressive.
This patch converts packet_line to packet_read_line_gently
to more clearly indicate its relationship to
packet_read_line, and renames the internal variable to
"gently". This is also not incredibly expressive, but it is
at least a convention within the git code. And finally, we
document the exact behavior for the gentle and non-gentle
modes.
While we are cleaning up the names, we can drop the
"return_line_fail" checks in packet_read_internal entirely.
They look like this:
ret = safe_read(..., return_line_fail);
if (return_line_fail && ret < 0)
...
The check for return_line_fail is a no-op; safe_read will
only ever return an error value if we passed it
return_line_fail in the first place.
Signed-off-by: Jeff King <peff@peff.net>
---
Obviously this one is a matter of taste, but I think the result is much
better. Certainly the documentation bits are hard to argue with. :)
connect.c | 2 +-
pkt-line.c | 16 ++++++++--------
pkt-line.h | 21 ++++++++++++++++++++-
3 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/connect.c b/connect.c
index 49e56ba..7e0920d 100644
--- a/connect.c
+++ b/connect.c
@@ -76,7 +76,7 @@ struct ref **get_remote_heads(int in, struct ref **list,
char *name;
int len, name_len;
- len = packet_read(in, buffer, sizeof(buffer));
+ len = packet_read_line_gently(in, buffer, sizeof(buffer));
if (len < 0)
die_initial_contact(got_at_least_one_head);
diff --git a/pkt-line.c b/pkt-line.c
index 699c2dd..62479d3 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -103,13 +103,13 @@ static int safe_read(int fd, void *buffer, unsigned size, int return_line_fail)
strbuf_add(buf, buffer, n);
}
-static int safe_read(int fd, void *buffer, unsigned size, int return_line_fail)
+static int safe_read(int fd, void *buffer, unsigned size, int gently)
{
ssize_t ret = read_in_full(fd, buffer, size);
if (ret < 0)
die_errno("read error");
else if (ret < size) {
- if (return_line_fail)
+ if (gently)
return -1;
die("The remote end hung up unexpectedly");
@@ -143,13 +143,13 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int return_
return len;
}
-static int packet_read_internal(int fd, char *buffer, unsigned size, int return_line_fail)
+static int packet_read_internal(int fd, char *buffer, unsigned size, int gently)
{
int len, ret;
char linelen[4];
- ret = safe_read(fd, linelen, 4, return_line_fail);
- if (return_line_fail && ret < 0)
+ ret = safe_read(fd, linelen, 4, gently);
+ if (ret < 0)
return ret;
len = packet_length(linelen);
if (len < 0)
@@ -161,15 +161,15 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int return_
len -= 4;
if (len >= size)
die("protocol error: bad line length %d", len);
- ret = safe_read(fd, buffer, len, return_line_fail);
- if (return_line_fail && ret < 0)
+ ret = safe_read(fd, buffer, len, gently);
+ if (ret < 0)
return ret;
buffer[len] = 0;
packet_trace(buffer, len, 0);
return len;
}
-int packet_read(int fd, char *buffer, unsigned size)
+int packet_read_line_gently(int fd, char *buffer, unsigned size)
{
return packet_read_internal(fd, buffer, size, 1);
}
diff --git a/pkt-line.h b/pkt-line.h
index 7a67e9c..31bd069 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -24,8 +24,27 @@ int packet_read_line(int fd, char *buffer, unsigned size);
void packet_buf_flush(struct strbuf *buf);
void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+/*
+ * Read a packetized line from the descriptor into the buffer. We will die
+ * under any of the following conditions:
+ *
+ * 1. Read error from descriptor.
+ *
+ * 2. Protocol error from the remote (e.g., bogus length characters).
+ *
+ * 3. Receiving a packet larger than "size" bytes.
+ *
+ * 4. Truncated output from the remote (e.g., we expected a packet but got
+ * EOF, or we got a partial packet followed by EOF).
+ */
int packet_read_line(int fd, char *buffer, unsigned size);
-int packet_read(int fd, char *buffer, unsigned size);
+
+/*
+ * Same as packet_read_line, but do not die on condition 4 (truncated input);
+ * instead return -1. We still die for the other conditions.
+ */
+int packet_read_line_gently(int fd, char *buffer, unsigned size);
+
int packet_get_line(struct strbuf *out, char **src_buf, size_t *src_len);
ssize_t safe_write(int, const void *, ssize_t);
--
1.8.1.20.g7078b03
^ permalink raw reply related
* [PATCHv2 04/10] pkt-line: change error message for oversized packet
From: Jeff King @ 2013-02-18 9:22 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
If we get a packet from the remote side that is too large to
fit in our buffer, we currently complain "protocol error:
bad line length". This is a bit vague. The line length the
other side sent is not "bad" per se; the actual problem is
that it exceeded our expectation for buffer length.
This will generally not happen between two git-core
implementations, because the sender limits themselves to
either 1000, or to LARGE_PACKET_MAX (depending on what is
being sent, sideband-64k negotiation, etc), and the receiver
uses a buffer of the appropriate size.
The protocol document indicates the LARGE_PACKET_MAX limit
(of 65520), but does not actually specify the 1000-byte
limit for ref lines. It is likely that other implementations
just create a packet as large as they need, and this doesn't
come up often because nobody has 1000-character ref names
(or close to it, counting sha1 and other boilerplate).
We may want to increase the size of our receive buffers for
ref lines to prepare for such writers (whether they are
other implementations, or if we eventually want to bump the
write size in git-core). In the meantime, this patch tries
to give a more clear message in case it does come up.
Signed-off-by: Jeff King <peff@peff.net>
---
I'm really tempted to bump all of our 1000-byte buffers to just use
LARGE_PACKET_MAX. If we provided a packet_read variant that used a
static buffer (which is fine for all but one or two callers), then it
would not take much memory (right now we stick some LARGE_PACKET_MAX
buffers on the stack, which is slightly questionable for
stack-restricted systems). But I left that for a different topic (and
even if we do, we would still want this message to catch anything over
the bizarre 65520 limit).
Out of curiosity, I grepped the list archives, and found only one
instance of this message. And it was somebody whose data stream was tainted
with random crud that happened to be numbers (much more common is "bad line
length character", when the crud does not look like a packet length).
pkt-line.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/pkt-line.c b/pkt-line.c
index 62479d3..f2a7575 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -160,7 +160,8 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int gently)
}
len -= 4;
if (len >= size)
- die("protocol error: bad line length %d", len);
+ die("protocol error: line too large: (expected %u, got %d)",
+ size, len);
ret = safe_read(fd, buffer, len, gently);
if (ret < 0)
return ret;
--
1.8.1.20.g7078b03
^ permalink raw reply related
* [PATCHv2 05/10] pkt-line: rename s/packet_read_line/packet_read/
From: Jeff King @ 2013-02-18 9:22 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
Originally packets were used just for the line-oriented ref
advertisement and negotiation. These days, we also stuff
packfiles and sidebands into them, and they do not
necessarily represent a line. Drop the "_line" suffix, as it
is not informative and makes the function names quite long
(especially as we add "_gently" and other variants).
Signed-off-by: Jeff King <peff@peff.net>
---
Again, this is a taste issue. Can be optional.
builtin/archive.c | 4 ++--
builtin/fetch-pack.c | 2 +-
builtin/receive-pack.c | 2 +-
builtin/upload-archive.c | 2 +-
connect.c | 2 +-
daemon.c | 2 +-
fetch-pack.c | 6 +++---
pkt-line.c | 4 ++--
pkt-line.h | 6 +++---
remote-curl.c | 6 +++---
send-pack.c | 4 ++--
sideband.c | 2 +-
upload-pack.c | 4 ++--
13 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/builtin/archive.c b/builtin/archive.c
index 9a1cfd3..bf600f5 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -53,7 +53,7 @@ static int run_remote_archiver(int argc, const char **argv,
packet_write(fd[1], "argument %s\n", argv[i]);
packet_flush(fd[1]);
- len = packet_read_line(fd[0], buf, sizeof(buf));
+ len = packet_read(fd[0], buf, sizeof(buf));
if (!len)
die(_("git archive: expected ACK/NAK, got EOF"));
if (buf[len-1] == '\n')
@@ -66,7 +66,7 @@ static int run_remote_archiver(int argc, const char **argv,
die(_("git archive: protocol error"));
}
- len = packet_read_line(fd[0], buf, sizeof(buf));
+ len = packet_read(fd[0], buf, sizeof(buf));
if (len)
die(_("git archive: expected a flush"));
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 940ae35..b59e60e 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -102,7 +102,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
*/
static char line[1000];
for (;;) {
- int n = packet_read_line(0, line, sizeof(line));
+ int n = packet_read(0, line, sizeof(line));
if (!n)
break;
if (line[n-1] == '\n')
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 48cd5dc..3f58ce8 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -736,7 +736,7 @@ static struct command *read_head_info(void)
char *refname;
int len, reflen;
- len = packet_read_line(0, line, sizeof(line));
+ len = packet_read(0, line, sizeof(line));
if (!len)
break;
if (line[len-1] == '\n')
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index b928beb..2ecf461 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -40,7 +40,7 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
sent_argv[0] = "git-upload-archive";
for (p = buf;;) {
/* This will die if not enough free space in buf */
- len = packet_read_line(0, p, (buf + sizeof buf) - p);
+ len = packet_read(0, p, (buf + sizeof buf) - p);
if (len == 0)
break; /* got a flush */
if (sent_argc > MAX_ARGS - 2)
diff --git a/connect.c b/connect.c
index 7e0920d..59266b1 100644
--- a/connect.c
+++ b/connect.c
@@ -76,7 +76,7 @@ struct ref **get_remote_heads(int in, struct ref **list,
char *name;
int len, name_len;
- len = packet_read_line_gently(in, buffer, sizeof(buffer));
+ len = packet_read_gently(in, buffer, sizeof(buffer));
if (len < 0)
die_initial_contact(got_at_least_one_head);
diff --git a/daemon.c b/daemon.c
index 4602b46..ac26b93 100644
--- a/daemon.c
+++ b/daemon.c
@@ -612,7 +612,7 @@ static int execute(void)
loginfo("Connection from %s:%s", addr, port);
alarm(init_timeout ? init_timeout : timeout);
- pktlen = packet_read_line(0, line, sizeof(line));
+ pktlen = packet_read(0, line, sizeof(line));
alarm(0);
len = strlen(line);
diff --git a/fetch-pack.c b/fetch-pack.c
index b90dadf..d89c9d0 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -173,7 +173,7 @@ static void consume_shallow_list(struct fetch_pack_args *args, int fd)
* is a block of have lines exchanged.
*/
char line[1000];
- while (packet_read_line(fd, line, sizeof(line))) {
+ while (packet_read(fd, line, sizeof(line))) {
if (!prefixcmp(line, "shallow "))
continue;
if (!prefixcmp(line, "unshallow "))
@@ -216,7 +216,7 @@ static enum ack_type get_ack(int fd, unsigned char *result_sha1)
static enum ack_type get_ack(int fd, unsigned char *result_sha1)
{
static char line[1000];
- int len = packet_read_line(fd, line, sizeof(line));
+ int len = packet_read(fd, line, sizeof(line));
if (!len)
die("git fetch-pack: expected ACK/NAK, got EOF");
@@ -350,7 +350,7 @@ static int find_common(struct fetch_pack_args *args,
unsigned char sha1[20];
send_request(args, fd[1], &req_buf);
- while (packet_read_line(fd[0], line, sizeof(line))) {
+ while (packet_read(fd[0], line, sizeof(line))) {
if (!prefixcmp(line, "shallow ")) {
if (get_sha1_hex(line + 8, sha1))
die("invalid shallow line: %s", line);
diff --git a/pkt-line.c b/pkt-line.c
index f2a7575..85faf73 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -170,12 +170,12 @@ int packet_read_line_gently(int fd, char *buffer, unsigned size)
return len;
}
-int packet_read_line_gently(int fd, char *buffer, unsigned size)
+int packet_read_gently(int fd, char *buffer, unsigned size)
{
return packet_read_internal(fd, buffer, size, 1);
}
-int packet_read_line(int fd, char *buffer, unsigned size)
+int packet_read(int fd, char *buffer, unsigned size)
{
return packet_read_internal(fd, buffer, size, 0);
}
diff --git a/pkt-line.h b/pkt-line.h
index 31bd069..2dc4941 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -37,13 +37,13 @@ int packet_read_line(int fd, char *buffer, unsigned size);
* 4. Truncated output from the remote (e.g., we expected a packet but got
* EOF, or we got a partial packet followed by EOF).
*/
-int packet_read_line(int fd, char *buffer, unsigned size);
+int packet_read(int fd, char *buffer, unsigned size);
/*
- * Same as packet_read_line, but do not die on condition 4 (truncated input);
+ * Same as packet_read, but do not die on condition 4 (truncated input);
* instead return -1. We still die for the other conditions.
*/
-int packet_read_line_gently(int fd, char *buffer, unsigned size);
+int packet_read_gently(int fd, char *buffer, unsigned size);
int packet_get_line(struct strbuf *out, char **src_buf, size_t *src_len);
ssize_t safe_write(int, const void *, ssize_t);
diff --git a/remote-curl.c b/remote-curl.c
index 7be4b53..99cc016 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -308,7 +308,7 @@ static size_t rpc_out(void *ptr, size_t eltsize,
if (!avail) {
rpc->initial_buffer = 0;
- avail = packet_read_line(rpc->out, rpc->buf, rpc->alloc);
+ avail = packet_read(rpc->out, rpc->buf, rpc->alloc);
if (!avail)
return 0;
rpc->pos = 0;
@@ -425,7 +425,7 @@ static int post_rpc(struct rpc_state *rpc)
break;
}
- n = packet_read_line(rpc->out, buf, left);
+ n = packet_read(rpc->out, buf, left);
if (!n)
break;
rpc->len += n;
@@ -579,7 +579,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
rpc->hdr_accept = strbuf_detach(&buf, NULL);
while (!err) {
- int n = packet_read_line(rpc->out, rpc->buf, rpc->alloc);
+ int n = packet_read(rpc->out, rpc->buf, rpc->alloc);
if (!n)
break;
rpc->pos = 0;
diff --git a/send-pack.c b/send-pack.c
index a99a1e4..7b22235 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -108,7 +108,7 @@ static int receive_status(int in, struct ref *refs)
struct ref *hint;
char line[1000];
int ret = 0;
- int len = packet_read_line(in, line, sizeof(line));
+ int len = packet_read(in, line, sizeof(line));
if (len < 10 || memcmp(line, "unpack ", 7))
return error("did not receive remote status");
if (memcmp(line, "unpack ok\n", 10)) {
@@ -122,7 +122,7 @@ static int receive_status(int in, struct ref *refs)
while (1) {
char *refname;
char *msg;
- len = packet_read_line(in, line, sizeof(line));
+ len = packet_read(in, line, sizeof(line));
if (!len)
break;
if (len < 3 ||
diff --git a/sideband.c b/sideband.c
index 8f7b25b..4a6446d 100644
--- a/sideband.c
+++ b/sideband.c
@@ -38,7 +38,7 @@ int recv_sideband(const char *me, int in_stream, int out)
while (1) {
int band, len;
- len = packet_read_line(in_stream, buf + pf, LARGE_PACKET_MAX);
+ len = packet_read(in_stream, buf + pf, LARGE_PACKET_MAX);
if (len == 0)
break;
if (len < 1) {
diff --git a/upload-pack.c b/upload-pack.c
index 04d6bd7..04231d7 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -428,7 +428,7 @@ static int get_common_commits(void)
save_commit_buffer = 0;
for (;;) {
- int len = packet_read_line(0, line, sizeof(line));
+ int len = packet_read(0, line, sizeof(line));
reset_timeout();
if (!len) {
@@ -589,7 +589,7 @@ static void receive_needs(void)
struct object *o;
const char *features;
unsigned char sha1_buf[20];
- len = packet_read_line(0, line, sizeof(line));
+ len = packet_read(0, line, sizeof(line));
reset_timeout();
if (!len)
break;
--
1.8.1.20.g7078b03
^ permalink raw reply related
* Re: git clone tag shallow
From: Junio C Hamano @ 2013-02-18 9:22 UTC (permalink / raw)
To: Thibault Kruse; +Cc: Duy Nguyen, git
In-Reply-To: <CAByu6UWO=kUOvJ_YcPG9bo+XVZ5hSxRQpyEaUMcVxa=sXt_EMw@mail.gmail.com>
Thibault Kruse <tibokruse@googlemail.com> writes:
> Whenever a command description involves "<branch>" this can, depending
> on the command, refer to
> 1) a name that, when prepended with "refs/heads/", is a valid ref,
> 2) a name that, when prepended with "refs/heads/" or "refs/tags", is a
> valid ref,
> 3) a name that, when prepended with "refs/[heads|tags]/", or unique in
> "refs/remotes/*/" is a valid ref
>
> Now in the docu I don't see a nice distinction between 1), 2) and 3).
> I could work on a patch if someone
> tells me how to clearly distinguish those cases.
It is _very_ true that we do not give strict distinction in many
cases in the SYNOPSIS section.
It is clear that (1) should use <branch> or even <branch-name>.
"git checkout master" and "git checkout head/master" mean very
different things. The former is the "git checkout <branch-name>"
case---checkout the named branch and prepare to grow the history of
that branch. The latter is "git checkout <committish>"---detach the
HEAD at that commit, and even when the committish was named using
the name of an existing branch (e.g. "master^0" or "heads/master"),
prevent future commits made in that state from affecting the branch.
I am not sure why you meant to treat (2) and (3) differently,
though. Care to elaborate?
And there is (4) that is not in your list.
A name that is not a local branch name (i.e. no refs/heads/$name)
and that there is only one ref that matches refs/remotes/*/$name,
such a name is special-cased in "git checkout $name". But I do not
know it is worth giving a name to such a narrow concept that is only
used for a single hacky special case. Whatever word you invent and
call such a name (perhaps "remote branch name"?), you would need to
repeat the first three lines of this paragraph in the description to
define that word anyway.
Outside "git checkout", we historically deliberately stayed loose in
an attempt to help beginners by avoiding <committish> or <ref>, when
most people are expected to feed branch names to the command and
used <branch>. I am not sure if it is a good idea to break such a
white lie just to be technically more correct in the first place.
It needs to be done with care to avoid making the resulting text
harder to approach for beginners.
^ permalink raw reply
* [PATCHv2 06/10] pkt-line: share buffer/descriptor reading implementation
From: Jeff King @ 2013-02-18 9:26 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
The packet_read function reads from a descriptor. The
packet_get_line function is similar, but reads from an
in-memory buffer, and uses a completely separate
implementation. This patch teaches the internal function
backing packet_read to accept either source, and use the
appropriate one. As a result:
1. The function has been renamed to packet_read_from_buf,
which more clearly indicates its relationship to the
packet_read function.
2. The original packet_get_line wrote to a strbuf; the new
function, like its descriptor counterpart, reads into a
buffer provided by the caller.
3. The original function did not die on any errors, but
instead returned an error code. Now we have the usual
"normal" and "gently" forms.
There are only two existing calls to packet_get_line which
have to be converted, and both are in remote-curl. The first
reads and checks the "# service=git-foo" line from a smart
http server. The second just reads past any additional
smart headers, without bothering to look at them.
This patch converts both to the new form, with a few
implications:
1. Because we use the non-gentle form, the first caller
can drop its own error checking. As a result, we will get
more accurate error reporting about protocol breakage,
since the errors come from inside the protocol code. We
will no longer print the URL as part of the error, but
that's OK. Protocol breakages should be rare (and we
are pretty sure at this point in the code that it is a
real smart server, so we won't be confused by dumb
servers), and the first debugging step would probably
be GIT_CURL_VERBOSE, anyway.
2. The second caller did not error check at all, and now
does. This can help us catch broken or truncated input
close to the source.
3. Since we are no longer using a strbuf, we now have a
1000-byte limit on the smart-http headers. That should
be fine, as the only header that has ever been sent
here is the short "service=git-foo" header.
Signed-off-by: Jeff King <peff@peff.net>
---
The diffstat shows more lines appearing, but it is mainly from comments
and from the various parse_line{_from_buf,}{,_gently} variants; we
really do get rid of a duplicate parsing implementation, and we
harmonize all of the error conditions and messages.
We can also make "gently" a parameter to avoid the proliferation of
related functions, but would mean all but one callsite would have to
pass an extra "0". Choose your poison, I guess.
pkt-line.c | 69 ++++++++++++++++++++++++++++-------------------------------
pkt-line.h | 11 +++++++++-
remote-curl.c | 19 ++++++++--------
3 files changed, 53 insertions(+), 46 deletions(-)
diff --git a/pkt-line.c b/pkt-line.c
index 85faf73..7ee91e0 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -103,12 +103,26 @@ static int safe_read(int fd, void *buffer, unsigned size, int gently)
strbuf_add(buf, buffer, n);
}
-static int safe_read(int fd, void *buffer, unsigned size, int gently)
+static int get_packet_data(int fd, char **src_buf, size_t *src_size,
+ void *dst, unsigned size, int gently)
{
- ssize_t ret = read_in_full(fd, buffer, size);
- if (ret < 0)
- die_errno("read error");
- else if (ret < size) {
+ ssize_t ret;
+
+ /* Read up to "size" bytes from our source, whatever it is. */
+ if (src_buf) {
+ ret = size < *src_size ? size : *src_size;
+ memcpy(dst, *src_buf, ret);
+ *src_buf += size;
+ *src_size -= size;
+ }
+ else {
+ ret = read_in_full(fd, dst, size);
+ if (ret < 0)
+ die_errno("read error");
+ }
+
+ /* And complain if we didn't get enough bytes to satisfy the read. */
+ if (ret < size) {
if (gently)
return -1;
@@ -143,12 +157,13 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int gently)
return len;
}
-static int packet_read_internal(int fd, char *buffer, unsigned size, int gently)
+static int packet_read_internal(int fd, char **src_buf, size_t *src_len,
+ char *buffer, unsigned size, int gently)
{
int len, ret;
char linelen[4];
- ret = safe_read(fd, linelen, 4, gently);
+ ret = get_packet_data(fd, src_buf, src_len, linelen, 4, gently);
if (ret < 0)
return ret;
len = packet_length(linelen);
@@ -162,7 +177,7 @@ static int packet_read_internal(int fd, char *buffer, unsigned size, int gently)
if (len >= size)
die("protocol error: line too large: (expected %u, got %d)",
size, len);
- ret = safe_read(fd, buffer, len, gently);
+ ret = get_packet_data(fd, src_buf, src_len, buffer, len, gently);
if (ret < 0)
return ret;
buffer[len] = 0;
@@ -172,40 +187,22 @@ int packet_get_line(struct strbuf *out,
int packet_read_gently(int fd, char *buffer, unsigned size)
{
- return packet_read_internal(fd, buffer, size, 1);
+ return packet_read_internal(fd, NULL, 0, buffer, size, 1);
}
int packet_read(int fd, char *buffer, unsigned size)
{
- return packet_read_internal(fd, buffer, size, 0);
+ return packet_read_internal(fd, NULL, 0, buffer, size, 0);
}
-int packet_get_line(struct strbuf *out,
- char **src_buf, size_t *src_len)
+int packet_read_from_buf(char *dst, unsigned dst_len,
+ char **src_buf, size_t *src_len)
{
- int len;
-
- if (*src_len < 4)
- return -1;
- len = packet_length(*src_buf);
- if (len < 0)
- return -1;
- if (!len) {
- *src_buf += 4;
- *src_len -= 4;
- packet_trace("0000", 4, 0);
- return 0;
- }
- if (*src_len < len)
- return -2;
-
- *src_buf += 4;
- *src_len -= 4;
- len -= 4;
+ return packet_read_internal(-1, src_buf, src_len, dst, dst_len, 0);
+}
- strbuf_add(out, *src_buf, len);
- *src_buf += len;
- *src_len -= len;
- packet_trace(out->buf, out->len, 0);
- return len;
+int packet_read_from_buf_gently(char *dst, unsigned dst_len,
+ char **src_buf, size_t *src_len)
+{
+ return packet_read_internal(-1, src_buf, src_len, dst, dst_len, 1);
}
diff --git a/pkt-line.h b/pkt-line.h
index 2dc4941..287a391 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -45,7 +45,16 @@ int packet_read_gently(int fd, char *buffer, unsigned size);
*/
int packet_read_gently(int fd, char *buffer, unsigned size);
-int packet_get_line(struct strbuf *out, char **src_buf, size_t *src_len);
+/*
+ * Same as packet_read above, but read from an in-memory buffer
+ * instead of a file descriptor. The src_buf and src_len are modified
+ * to iterate past the consumed data.
+ */
+int packet_read_from_buf(char *dst, unsigned dst_len,
+ char **src_buf, size_t *src_len);
+int packet_read_from_buf_gently(char *dst, unsigned dst_len,
+ char **src_buf, size_t *src_len);
+
ssize_t safe_write(int, const void *, ssize_t);
#endif
diff --git a/remote-curl.c b/remote-curl.c
index 99cc016..2ec5854 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -138,28 +138,29 @@ static struct discovery* discover_refs(const char *service)
if (maybe_smart &&
(5 <= last->len && last->buf[4] == '#') &&
!strbuf_cmp(&exp, &type)) {
+ char line[1000];
+ int len;
+
/*
* smart HTTP response; validate that the service
* pkt-line matches our request.
*/
- if (packet_get_line(&buffer, &last->buf, &last->len) <= 0)
- die("%s has invalid packet header", refs_url);
- if (buffer.len && buffer.buf[buffer.len - 1] == '\n')
- strbuf_setlen(&buffer, buffer.len - 1);
+ len = packet_read_from_buf(line, sizeof(line), &last->buf, &last->len);
+ if (len && line[len - 1] == '\n')
+ len--;
strbuf_reset(&exp);
strbuf_addf(&exp, "# service=%s", service);
- if (strbuf_cmp(&exp, &buffer))
- die("invalid server response; got '%s'", buffer.buf);
+ if (len != exp.len || memcmp(exp.buf, line, len))
+ die("invalid server response; got '%s'", line);
strbuf_release(&exp);
/* The header can include additional metadata lines, up
* until a packet flush marker. Ignore these now, but
* in the future we might start to scan them.
*/
- strbuf_reset(&buffer);
- while (packet_get_line(&buffer, &last->buf, &last->len) > 0)
- strbuf_reset(&buffer);
+ while (packet_read_from_buf(line, sizeof(line), &last->buf, &last->len) > 0)
+ ;
last->proto_git = 1;
}
--
1.8.1.20.g7078b03
^ permalink raw reply related
* [PATCHv2 07/10] teach get_remote_heads to read from a memory buffer
From: Jeff King @ 2013-02-18 9:27 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
Now that we can read packet data from memory as easily as a
descriptor, get_remote_heads can take either one as a
source. This will allow further refactoring in remote-curl.
Signed-off-by: Jeff King <peff@peff.net>
---
There aren't that many callers of get_remote_heads, so I just added the
optional parameters and tweaked each callsite. We can do it as a
get_remote_heads_from_buf wrapper function and leave them be, if we
want.
builtin/fetch-pack.c | 2 +-
builtin/send-pack.c | 2 +-
cache.h | 4 +++-
connect.c | 10 +++++++---
remote-curl.c | 2 +-
transport.c | 6 +++---
6 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index b59e60e..bf850d0 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -128,7 +128,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
args.verbose ? CONNECT_VERBOSE : 0);
}
- get_remote_heads(fd[0], &ref, 0, NULL);
+ get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL);
ref = fetch_pack(&args, fd, conn, ref, dest,
&sought, pack_lockfile_ptr);
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 8778519..152c4ea 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -207,7 +207,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
memset(&extra_have, 0, sizeof(extra_have));
- get_remote_heads(fd[0], &remote_refs, REF_NORMAL, &extra_have);
+ get_remote_heads(fd[0], NULL, 0, &remote_refs, REF_NORMAL, &extra_have);
transport_verify_remote_names(nr_refspecs, refspecs);
diff --git a/cache.h b/cache.h
index e493563..db646a2 100644
--- a/cache.h
+++ b/cache.h
@@ -1049,7 +1049,9 @@ struct extra_have_objects {
int nr, alloc;
unsigned char (*array)[20];
};
-extern struct ref **get_remote_heads(int in, struct ref **list, unsigned int flags, struct extra_have_objects *);
+extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
+ struct ref **list, unsigned int flags,
+ struct extra_have_objects *);
extern int server_supports(const char *feature);
extern int parse_feature_request(const char *features, const char *feature);
extern const char *server_feature_value(const char *feature, int *len_ret);
diff --git a/connect.c b/connect.c
index 59266b1..28f21d0 100644
--- a/connect.c
+++ b/connect.c
@@ -62,8 +62,8 @@ static void die_initial_contact(int got_at_least_one_head)
/*
* Read all the refs from the other end
*/
-struct ref **get_remote_heads(int in, struct ref **list,
- unsigned int flags,
+struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
+ struct ref **list, unsigned int flags,
struct extra_have_objects *extra_have)
{
int got_at_least_one_head = 0;
@@ -76,7 +76,11 @@ struct ref **get_remote_heads(int in, struct ref **list,
char *name;
int len, name_len;
- len = packet_read_gently(in, buffer, sizeof(buffer));
+ if (src_buf)
+ len = packet_read_from_buf_gently(buffer, sizeof(buffer),
+ &src_buf, &src_len);
+ else
+ len = packet_read_gently(in, buffer, sizeof(buffer));
if (len < 0)
die_initial_contact(got_at_least_one_head);
diff --git a/remote-curl.c b/remote-curl.c
index 2ec5854..6e43463 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -195,7 +195,7 @@ static struct ref *parse_git_refs(struct discovery *heads, int for_push)
if (start_async(&async))
die("cannot start thread to parse advertised refs");
- get_remote_heads(async.out, &list,
+ get_remote_heads(async.out, NULL, 0, &list,
for_push ? REF_NORMAL : 0, NULL);
close(async.out);
if (finish_async(&async))
diff --git a/transport.c b/transport.c
index 886ffd8..62df466 100644
--- a/transport.c
+++ b/transport.c
@@ -507,7 +507,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
struct ref *refs;
connect_setup(transport, for_push, 0);
- get_remote_heads(data->fd[0], &refs,
+ get_remote_heads(data->fd[0], NULL, 0, &refs,
for_push ? REF_NORMAL : 0, &data->extra_have);
data->got_remote_heads = 1;
@@ -541,7 +541,7 @@ static int fetch_refs_via_pack(struct transport *transport,
if (!data->got_remote_heads) {
connect_setup(transport, 0, 0);
- get_remote_heads(data->fd[0], &refs_tmp, 0, NULL);
+ get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0, NULL);
data->got_remote_heads = 1;
}
@@ -799,7 +799,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
struct ref *tmp_refs;
connect_setup(transport, 1, 0);
- get_remote_heads(data->fd[0], &tmp_refs, REF_NORMAL, NULL);
+ get_remote_heads(data->fd[0], NULL, 0, &tmp_refs, REF_NORMAL, NULL);
data->got_remote_heads = 1;
}
--
1.8.1.20.g7078b03
^ permalink raw reply related
* [PATCHv2 08/10] remote-curl: pass buffer straight to get_remote_heads
From: Jeff King @ 2013-02-18 9:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
Until recently, get_remote_heads only knew how to read refs
from a file descriptor. To hack around this, we spawned a
thread (or forked a process) to write the buffer back to us.
Now that we can just pass it our buffer directly, we don't
have to use this hack anymore.
Signed-off-by: Jeff King <peff@peff.net>
---
I don't know that this code was hurting anything, but it has always
struck me as ugly and a possible source of error. And now it's gone.
remote-curl.c | 26 ++------------------------
1 file changed, 2 insertions(+), 24 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 6e43463..f049da2 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -173,33 +173,11 @@ static struct ref *parse_git_refs(struct discovery *heads, int for_push)
return last;
}
-static int write_discovery(int in, int out, void *data)
-{
- struct discovery *heads = data;
- int err = 0;
- if (write_in_full(out, heads->buf, heads->len) != heads->len)
- err = 1;
- close(out);
- return err;
-}
-
static struct ref *parse_git_refs(struct discovery *heads, int for_push)
{
struct ref *list = NULL;
- struct async async;
-
- memset(&async, 0, sizeof(async));
- async.proc = write_discovery;
- async.data = heads;
- async.out = -1;
-
- if (start_async(&async))
- die("cannot start thread to parse advertised refs");
- get_remote_heads(async.out, NULL, 0, &list,
- for_push ? REF_NORMAL : 0, NULL);
- close(async.out);
- if (finish_async(&async))
- die("ref parsing thread failed");
+ get_remote_heads(-1, heads->buf, heads->len, &list,
+ for_push ? REF_NORMAL : 0, NULL);
return list;
}
--
1.8.1.20.g7078b03
^ permalink raw reply related
* Re: [PATCHv2 0/10] pkt-line and remote-curl cleanups server
From: Junio C Hamano @ 2013-02-18 9:29 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Feb 17, 2013 at 05:41:13PM -0800, Jonathan Nieder wrote:
>
>> > I don't think so. Don't ERR lines appear inside their own packets?
>>
>> Yes, I misread get_remote_heads for some reason. Thanks for checking.
>
> Thanks for bringing it up. I had not even thought about ERR at all. So
> it was luck rather than skill that I was right. :)
>
>> I'm not sure whether servers are expected to send a flush after an
>> ERR packet. The only codepath I know of in git itself that sends
>> such packets is git-daemon, which does not flush after the error (but
>> is not used in the stateless-rpc case). http-backend uses HTTP error
>> codes for its errors.
>
> I just checked, and GitHub also does not send flush packets after ERR.
> Which makes sense; ERR is supposed to end the conversation.
Hmph. A flush packet was supposed to be a mark to say "all the
packets before this one can be buffered and kept without getting
passed to write(2), but this and all such buffered data _must_ go on
the wire _now_". So in the sense, ERR not followed by a flush may
not even have a chance to be seen on the other end, no? That is
what the comment before the implementation of packet_flush() is all
about.
^ permalink raw reply
* [PATCHv2 09/10] remote-curl: move ref-parsing code up in file
From: Jeff King @ 2013-02-18 9:29 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Shawn O. Pearce
In-Reply-To: <20130218091203.GB17003@sigill.intra.peff.net>
The ref-parsing functions are static. Let's move them up in
the file to be available to more functions, which will help
us with later refactoring.
Signed-off-by: Jeff King <peff@peff.net>
---
Just a cleanup for the next patch. We could also just do extra
declarations at the top.
remote-curl.c | 117 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 59 insertions(+), 58 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index f049da2..62f82d1 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -80,6 +80,65 @@ static struct discovery *last_discovery;
};
static struct discovery *last_discovery;
+static struct ref *parse_git_refs(struct discovery *heads, int for_push)
+{
+ struct ref *list = NULL;
+ get_remote_heads(-1, heads->buf, heads->len, &list,
+ for_push ? REF_NORMAL : 0, NULL);
+ return list;
+}
+
+static struct ref *parse_info_refs(struct discovery *heads)
+{
+ char *data, *start, *mid;
+ char *ref_name;
+ int i = 0;
+
+ struct ref *refs = NULL;
+ struct ref *ref = NULL;
+ struct ref *last_ref = NULL;
+
+ data = heads->buf;
+ start = NULL;
+ mid = data;
+ while (i < heads->len) {
+ if (!start) {
+ start = &data[i];
+ }
+ if (data[i] == '\t')
+ mid = &data[i];
+ if (data[i] == '\n') {
+ if (mid - start != 40)
+ die("%sinfo/refs not valid: is this a git repository?", url);
+ data[i] = 0;
+ ref_name = mid + 1;
+ ref = xmalloc(sizeof(struct ref) +
+ strlen(ref_name) + 1);
+ memset(ref, 0, sizeof(struct ref));
+ strcpy(ref->name, ref_name);
+ get_sha1_hex(start, ref->old_sha1);
+ if (!refs)
+ refs = ref;
+ if (last_ref)
+ last_ref->next = ref;
+ last_ref = ref;
+ start = NULL;
+ }
+ i++;
+ }
+
+ ref = alloc_ref("HEAD");
+ if (!http_fetch_ref(url, ref) &&
+ !resolve_remote_symref(ref, refs)) {
+ ref->next = refs;
+ refs = ref;
+ } else {
+ free(ref);
+ }
+
+ return refs;
+}
+
static void free_discovery(struct discovery *d)
{
if (d) {
@@ -173,64 +232,6 @@ static struct discovery* discover_refs(const char *service)
return last;
}
-static struct ref *parse_git_refs(struct discovery *heads, int for_push)
-{
- struct ref *list = NULL;
- get_remote_heads(-1, heads->buf, heads->len, &list,
- for_push ? REF_NORMAL : 0, NULL);
- return list;
-}
-
-static struct ref *parse_info_refs(struct discovery *heads)
-{
- char *data, *start, *mid;
- char *ref_name;
- int i = 0;
-
- struct ref *refs = NULL;
- struct ref *ref = NULL;
- struct ref *last_ref = NULL;
-
- data = heads->buf;
- start = NULL;
- mid = data;
- while (i < heads->len) {
- if (!start) {
- start = &data[i];
- }
- if (data[i] == '\t')
- mid = &data[i];
- if (data[i] == '\n') {
- if (mid - start != 40)
- die("%sinfo/refs not valid: is this a git repository?", url);
- data[i] = 0;
- ref_name = mid + 1;
- ref = xmalloc(sizeof(struct ref) +
- strlen(ref_name) + 1);
- memset(ref, 0, sizeof(struct ref));
- strcpy(ref->name, ref_name);
- get_sha1_hex(start, ref->old_sha1);
- if (!refs)
- refs = ref;
- if (last_ref)
- last_ref->next = ref;
- last_ref = ref;
- start = NULL;
- }
- i++;
- }
-
- ref = alloc_ref("HEAD");
- if (!http_fetch_ref(url, ref) &&
- !resolve_remote_symref(ref, refs)) {
- ref->next = refs;
- refs = ref;
- } else {
- free(ref);
- }
-
- return refs;
-}
static struct ref *get_refs(int for_push)
{
--
1.8.1.20.g7078b03
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox