Re: [PATCH 1/2] mailinfo: Add support for keep_cr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/12/2017 01:20 AM, Matthew Wilcox wrote:
From: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>

If you have a base-64 encoded patch with CRLF endings (as produced
by forwarding a patch from Outlook to a Linux machine, for example),
the keep_cr setting is not honoured because keep_cr is only passed
to mailsplit, which does not look through the encoding.  The keep_cr
logic needs to be applied after the base64 decode.  I copied that
logic to handle_filter(), and rather than add a new keep_cr parameter
to handle_filter, I opted to add keep_cr to struct mailinfo; it seemed
appropriate given use_scissors was already there.

Then I needed to initialise keep_cr in the struct mailinfo passed from
git-am, and rather than thread a 'keep_cr' parameter all the way through
to parse_mail(), I decided to add keep_cr to struct am_state, which let
it be removed as a parameter from five other functions.

Signed-off-by: Matthew Wilcox <mawilcox@xxxxxxxxxxxxx>

A test exercising the new functionality would be nice.

Also, maybe a more descriptive title like "mailinfo: also respect keep_cr after base64 decode" (50 characters) is better.

@@ -143,6 +144,7 @@ static void am_state_init(struct am_state *state, const char *dir)

 	memset(state, 0, sizeof(*state));

+	state->keep_cr = -1;

Maybe query the git config here (instead of later) so that we never have to worry about state->keep_cr being neither 0 nor 1? This function already queries the git config anyway.

diff --git a/mailinfo.h b/mailinfo.h
index 04a25351d..9fddcf684 100644
--- a/mailinfo.h
+++ b/mailinfo.h
@@ -12,6 +12,7 @@ struct mailinfo {
 	struct strbuf email;
 	int keep_subject;
 	int keep_non_patch_brackets_in_subject;
+	int keep_cr;
 	int add_message_id;
 	int use_scissors;
 	int use_inbody_headers;

I personally would write "unsigned keep_cr : 1" to further emphasize that this can only be 0 or 1, but I don't know if it's better to keep with the style existing in the file (that is, using int).



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]