git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 06/14] imap-send.c: remove some unused fields from struct store

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Jan 14, 2013, 09:25 UTC
Message-ID
<50F3CF10.3000602@alum.mit.edu>
In-Reply-To
<20130114061920.GE3125@elie.Belkin>
On 01/14/2013 07:19 AM, Jonathan Nieder wrote:
Show 42 quoted lines
> 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/
Previous: Jonathan NiederNext: Michael Haggerty
Message 11 of 27 in “Remove unused code from imap-send.c”
  1. 00/14 Remove unused code from imap-send.cMichael Haggerty, Jan 14, 2013
  2. 01/14 imap-send.c: remove msg_data::flags, which was always zeroMichael Haggerty, Jan 14, 2013
  3. Jonathan NiederJan 14, 2013
  4. Michael HaggertyJan 14, 2013
  5. 02/14 imap-send.c: remove struct msg_dataMichael Haggerty, Jan 14, 2013
  6. 03/14 iamp-send.c: remove unused struct imap_store_confMichael Haggerty, Jan 14, 2013
  7. 04/14 imap-send.c: remove struct store_confMichael Haggerty, Jan 14, 2013
  8. 05/14 imap-send.c: remove struct messageMichael Haggerty, Jan 14, 2013
  9. 06/14 imap-send.c: remove some unused fields from struct storeMichael Haggerty, Jan 14, 2013
  10. Jonathan NiederJan 14, 2013
  11. Michael HaggertyJan 14, 2013
  12. 07/14 imap-send.c: inline imap_parse_list() in imap_list()Michael Haggerty, Jan 14, 2013
  13. 08/14 imap-send.c: remove struct imap argument to parse_imap_list_l()Michael Haggerty, Jan 14, 2013
  14. 09/14 imap-send.c: remove namespace fields from struct imapMichael Haggerty, Jan 14, 2013
  15. Jonathan NiederJan 14, 2013
  16. Michael HaggertyJan 14, 2013
  17. 10/14 imap-send.c: remove unused field imap_store::trashncMichael Haggerty, Jan 14, 2013
  18. 11/14 imap-send.c: simplify logic in lf_to_crlf()Michael Haggerty, Jan 14, 2013
  19. Jonathan NiederJan 14, 2013
  20. 12/14 imap-send.c: use struct imap_store instead of struct storeMichael Haggerty, Jan 14, 2013
  21. Jonathan NiederJan 14, 2013
  22. 13/14 imap-send.c: remove unused field imap_store::uidvalidityMichael Haggerty, Jan 14, 2013
  23. 14/14 imap-send.c: fold struct store into struct imap_storeMichael Haggerty, Jan 14, 2013
  24. Jeff KingJan 14, 2013
  25. Jonathan NiederJan 14, 2013
  26. Michael HaggertyJan 14, 2013
  27. Junio C HamanoJan 14, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.