Re: [PATCH] git-send-email.perl: check for lines longer than 998 characters

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

 



On Fri, Jan 18, 2008 at 12:57:41PM -0800, Junio C Hamano wrote:

> >   2/3: send-email: validate patches before sending anything
> >
> >        The actual up front long-lines check.
> 
> I wonder what the performance implication of this approach would
> be, though.  I am tempted to say that it would be negligible --
> scanning text in Perl is fast enough.

We now open and do one conditional per line for each file (in addition
to already going through each file a separate time and doing more
complex processing).  Doing that over the entirety of "git log
--pretty=email -p" on git.git takes about 1 second on my machine for
11402 patches.  Obviously there's slightly more syscall overhead as you
have to open() each patch, but I think think it is clear that the
parsing overhead is negligible.

> I suspect that taking this "Safe against SMTP line length limit"
> topic all the way ("all the way" is post 1.5.4, I am inclined to
> agree that this may be a good fix to an existing bug) would
> require that git-format-patch --attach to learn to apply QP on
> patch text to avoid producing very long lines to root-cause the
> issue [*1*].

Perhaps. If such things are sufficiently rare, one could simply attach
the patch in their MUA. I think the most important thing is for git to
at least stop and warn the user that it might not be sending something
valid. But implementing N different fixes that haven't even been
requested by users seems like a waste of time.

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

  Powered by Linux