Re: [PATCH 2/5] add the ability to select more email header fields to output

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

 



Don Zickus <dzickus@xxxxxxxxxx> writes:

> This is useful when scripts need more than just the basic email headers to
> parse.  By specifying the "-x=" option, one can search and output any header
> field they want.

It probably is useful, but that is rather difficult to judge,
unless you have a specific use in the scripts (am/applymox).

> @@ -870,6 +871,8 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
>  	def_charset = (git_commit_encoding ? git_commit_encoding : "utf-8");
>  	metainfo_charset = def_charset;
>  
> +	for (top=0; header[top]; top++){ ; }
> +
>  	while (1 < argc && argv[1][0] == '-') {
>  		if (!strcmp(argv[1], "-k"))
>  			keep_subject = 1;
> @@ -879,7 +882,10 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
>  			metainfo_charset = NULL;
>  		else if (!prefixcmp(argv[1], "--encoding="))
>  			metainfo_charset = argv[1] + 11;
> -		else
> +		else if (!prefixcmp(argv[1], "-x=")) {
> +			header[top] = xmalloc(256*sizeof(char));
> +			strncpy(header[top++], argv[1]+3, 256);

Return "-ETOOMANYEXTRAHEADERSHEADERS" when top overflows,
perhaps?

You seem to omit SP around '=' in initializers (the first part
of for loop and "typename var=init" at the beginning of block)
but not in an assignment expression used as a freestanding
statement.  Is this recommended by some coding style I am not
aware of, or it it just your habit?  It is somewhat irritating
to my eyes, although they might be syntactically different class
and you might be using one from the other consistently (but in
[1/5] some SP around '=' in assignments are omitted, and there
does not seem to be any such consistency).

And a micronit on [1/5] in the series.  I do not think "less
than zero" comment applies to what is being done, and I do not
think it needs to be explained what the code is doing by
checking return value from strcasestr() with NULL.

+	char boundary[256];
+
+	/* the only time this return less than zero is when 
+	   /line/ does not contain "text/"
 	 */
-	if (strcasestr(line, "boundary=")) {
-		fprintf(stderr, "Not handling nested multipart message.\n");
-		exit(1);
+	if (strcasestr(line, "text/") == NULL)
+		 message_type = TYPE_OTHER;

I think 3, 4, and 5 are good changes too.  Will apply with the
abovementioned micronit fixups and let's cook them in 'next'.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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]