From: Michael Haggerty <mhagger@alum.mit.edu>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
git@vger.kernel.org
Subject: Re: [PATCH 06/14] imap-send.c: remove some unused fields from struct store
Date: Mon, 14 Jan 2013 10:25:36 +0100 [thread overview]
Message-ID: <50F3CF10.3000602@alum.mit.edu> (raw)
In-Reply-To: <20130114061920.GE3125@elie.Belkin>
On 01/14/2013 07:19 AM, Jonathan Nieder wrote:
> Michael Haggerty wrote:
>
>> --- a/imap-send.c
>> +++ b/imap-send.c
> [...]
>> @@ -772,13 +767,10 @@ static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)
>> !strcmp("NO", arg) || !strcmp("BYE", arg)) {
>> if ((resp = parse_response_code(ctx, NULL, cmd)) != RESP_OK)
>> return resp;
>> - } else if (!strcmp("CAPABILITY", arg))
>> + } else if (!strcmp("CAPABILITY", arg)) {
>> parse_capability(imap, cmd);
>> - else if ((arg1 = next_arg(&cmd))) {
>> - if (!strcmp("EXISTS", arg1))
>> - ctx->gen.count = atoi(arg);
>> - else if (!strcmp("RECENT", arg1))
>> - ctx->gen.recent = atoi(arg);
>> + } else if ((arg1 = next_arg(&cmd))) {
>> + /* unused */
>
> Neat. Let me try to understand what was going on here:
>
> When opening a mailbox with the SELECT command, an IMAP server
> responds with tagged data indicating how many messages exist and how
> many are marked Recent. But git imap-send never reads mailboxes and
> in particular never uses the SELECT command, so there is no need for
> us to parse or record such responses.
>
> Out of paranoia we are keeping the parsing for now, but the parsed
> response is unused, hence the comment above.
>
> If I've understood correctly so far (a big assumption), I still am not
> sure what it would mean if we hit this ((arg1 = next_arg(&cmd))) case.
> Does it mean:
>
> A. The server has gone haywire and given a tagged response where
> one is not allowed, but let's tolerate it because we always have
> done so? Or
>
> B. This is a perfectly normal response to some of the commands we
> send, and we have always been deliberately ignoring it because it
> is not important for what imap-send does?
Honestly, I didn't bother to look into this. I was just doing some
brainless elimination of obviously unused code.
No doubt a deeper analysis (like yours) could find more code to discard,
but I didn't want to invest that much time and this code has absolutely
no tests, so I stuck to the obvious (and even then you found a mistake
in my changes :-( ).
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
next prev parent reply other threads:[~2013-01-14 9:26 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-14 5:32 [PATCH 00/14] Remove unused code from imap-send.c Michael Haggerty
2013-01-14 5:32 ` [PATCH 01/14] imap-send.c: remove msg_data::flags, which was always zero Michael Haggerty
2013-01-14 5:57 ` Jonathan Nieder
2013-01-14 9:10 ` Michael Haggerty
2013-01-14 5:32 ` [PATCH 02/14] imap-send.c: remove struct msg_data Michael Haggerty
2013-01-14 5:32 ` [PATCH 03/14] iamp-send.c: remove unused struct imap_store_conf Michael Haggerty
2013-01-14 5:32 ` [PATCH 04/14] imap-send.c: remove struct store_conf Michael Haggerty
2013-01-14 5:32 ` [PATCH 05/14] imap-send.c: remove struct message Michael Haggerty
2013-01-14 5:32 ` [PATCH 06/14] imap-send.c: remove some unused fields from struct store Michael Haggerty
2013-01-14 6:19 ` Jonathan Nieder
2013-01-14 9:25 ` Michael Haggerty [this message]
2013-01-14 5:32 ` [PATCH 07/14] imap-send.c: inline imap_parse_list() in imap_list() Michael Haggerty
2013-01-14 5:32 ` [PATCH 08/14] imap-send.c: remove struct imap argument to parse_imap_list_l() Michael Haggerty
2013-01-14 5:32 ` [PATCH 09/14] imap-send.c: remove namespace fields from struct imap Michael Haggerty
2013-01-14 6:43 ` Jonathan Nieder
2013-01-14 9:31 ` Michael Haggerty
2013-01-14 5:32 ` [PATCH 10/14] imap-send.c: remove unused field imap_store::trashnc Michael Haggerty
2013-01-14 5:32 ` [PATCH 11/14] imap-send.c: simplify logic in lf_to_crlf() Michael Haggerty
2013-01-14 6:47 ` Jonathan Nieder
2013-01-14 5:32 ` [PATCH 12/14] imap-send.c: use struct imap_store instead of struct store Michael Haggerty
2013-01-14 6:52 ` Jonathan Nieder
2013-01-14 5:32 ` [PATCH 13/14] imap-send.c: remove unused field imap_store::uidvalidity Michael Haggerty
2013-01-14 5:32 ` [PATCH 14/14] imap-send.c: fold struct store into struct imap_store Michael Haggerty
2013-01-14 6:06 ` [PATCH 00/14] Remove unused code from imap-send.c Jeff King
2013-01-14 6:57 ` Jonathan Nieder
2013-01-14 9:33 ` Michael Haggerty
2013-01-14 19:02 ` 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=50F3CF10.3000602@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=peff@peff.net \
/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.