Re: [PATCH 3/3] introduce "format" date-mode

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

 



On Tue, Jun 30, 2015 at 12:58:33PM -0400, Eric Sunshine wrote:

> > Basically I was trying to avoid making any assumptions about exactly how
> > strftime works. But presumably "stick a space in the format" is a
> > universally reasonable thing to do. It's a hack, but it's contained to
> > the function.
> 
> I don't think we're making any assumptions about strftime(). POSIX states:
> 
>     The format string consists of zero or more conversion
>     specifications and ordinary characters. [...] All ordinary
>     characters (including the terminating NUL character) are copied
>     unchanged into the array.
> 
> So, we seem to be on solid footing with this approach (even though
> it's a localized hack).

Yeah, sorry I wasn't more clear. I had originally been thinking of
making assumptions like "well, %c cannot ever be blank". But your
solution does not suffer from that level of knowledge. I think it is
reasonably clever.

> Yeah, I toyed with the idea of increasing the requested amount each
> iteration but wanted to keep the example simple, thus left it out.
> However, for some reason, I was thinking that strbuf_grow() was
> unconditionally expanding the buffer by the requested amount rather
> than merely ensuring that that amount was availabile, so the amount
> clearly needs to be increased on each iteration. Thanks for pointing
> that out.

FWIW, I had to look at it to double-check. I've often made the same
mistake.

> Beyond the extra allocation, I was also concerned about the
> sledgehammer approach of "%s " to append a single character when there
> are much less expensive ways to do so.

I don't think there's any other way. We have to feed a contiguous buffer
to strftime, and we don't own the buffer, so we have to make a new copy.

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