Git's list of banned C functions • Hacker News Board

Re: Git's list of banned C functions

#11

Post by captainmuon » Thu, Mar 04, 2021, 8:54 PM UTC

It would be interesting to see the rationale behind these bans, and what the suggested alternatives are. Some are obvious, like `strcpy`, but I can't remember what the problem with `sprintf` or the time functions are.

If you are doing something like `sprintf(buffer, "%f, %f", a, b)`, yes it is tricky to choose the size of buffer frugally, but if you replace that by `ftoa` and constructing the string by hand, you are likely to introduce more bugs.

Edit: as pointed out in another post, you can do git blame to see the rationale for each ban, quite interesing.

Re: Git's list of banned C functions

#12

Post by radus » Thu, Mar 04, 2021, 8:54 PM UTC

bvaldivielso wrote: post #4

Ah this is a very good idea. I guess you still have to make sure that all your translation units include this header, which isn't completely foolproof. Static analysis would probably be more robust, but way more involved.

Best of both worlds: use static analysis to ensure the header is included?

Re: Git's list of banned C functions

#13

Post by cle » Thu, Mar 04, 2021, 8:54 PM UTC

drfuchs wrote: post #6

It would be nice if the error messages generated would suggest replacement functions that they deem appropriate. I see that I'm not supposed to use gmtime, localtime, ctime, ctime_r, asctime, and asctime_r; but what do they think I should use?

From the commit messages

> The ctime_r() and asctime_r() functions are reentrant, but have no check that the buffer we pass in is long enough (the manpage says it "should have room for at least 26 bytes"). Since this is such an easy-to-get-wrong interface, and since we have the much safer strftime() as well as its more convenient strbuf_addftime() wrapper, let's ban both of those.

(https://github.com/git/git/commit/91aef030152d121f6b4bc3b933...)

> The traditional gmtime(), localtime(), ctime(), and asctime() functions return pointers to shared storage. This means they're not thread-safe, and they also run the risk of somebody holding onto the result across multiple calls (where each call invalidates the previous result). All callers should be using their reentrant counterparts.

(https://github.com/git/git/commit/1fbfdf556f2abc708183caca53...)

Re: Git's list of banned C functions

#14

Post by [deleted] » Thu, Mar 04, 2021, 8:55 PM UTC

drfuchs wrote: post #6

It would be nice if the error messages generated would suggest replacement functions that they deem appropriate. I see that I'm not supposed to use gmtime, localtime, ctime, ctime_r, asctime, and asctime_r; but what do they think I should use?

[deleted]

Re: Git's list of banned C functions

#15

Post by chris_wot » Thu, Mar 04, 2021, 8:55 PM UTC

drfuchs wrote: post #6

It would be nice if the error messages generated would suggest replacement functions that they deem appropriate. I see that I'm not supposed to use gmtime, localtime, ctime, ctime_r, asctime, and asctime_r; but what do they think I should use?

The commits actually do give that info. Take for instance this commit:

https://github.com/git/git/commit/c8af66ab8ad7cd78557f0f9f5e...

It actually gives examples and a lengthy explanation and reasoning behind the ban.

Re: Git's list of banned C functions

#16

Post by attractivechaos » Thu, Mar 04, 2021, 8:55 PM UTC

I wonder how they copy strings with strcpy and strncpy both banned. strlcpy? But it is not conforming to major standards. Or just memcpy with extra code?

Re: Git's list of banned C functions

#17

Post by paultopia » Thu, Mar 04, 2021, 8:55 PM UTC

Its really wild, as a person coming from other languages who has written maybe ten lines of C in his life that the functions that seem to be massive footguns in C are, like, "format a string" or "get time in GMT." That's... really scary.

Re: Git's list of banned C functions

#18

Post by edflsafoiewq » Thu, Mar 04, 2021, 8:55 PM UTC

alexchamberlain wrote: post #7

zbendefy wrote: post #3

Are there some details on whats wrong with these?

All the string functions have buffer overrun vulnerabilities if not used carefully. I'm not sure about the time functions though.

The time functions are either non-reentrant, or, for the _r versions, have the same problem with buffer overruns.

https://github.com/git/git/commit/1fbfdf556f2abc708183caca53...

https://github.com/git/git/commit/91aef030152d121f6b4bc3b933...

Re: Git's list of banned C functions

#19

Post by drfuchs » Thu, Mar 04, 2021, 8:56 PM UTC

cle wrote: post #13

drfuchs wrote: post #6

It would be nice if the error messages generated would suggest replacement functions that they deem appropriate. I see that I'm not supposed to use gmtime, localtime, ctime, ctime_r, asctime, and asctime_r; but what do they think I should use?

From the commit messages > The ctime_r() and asctime_r() functions are reentrant, but have no check that the buffer we pass in is long enough (the manpage says it "should have room for at least 26 bytes"). Since this is such an easy-to-get-wrong interface, and since we have the much safer strftime() as well as its more convenient strbuf_addftime() wrapper, let's ban both of those. ( https://github.com/git/git/commit/…

Yes, but every hapless user shouldn't have to go searching through a bunch of commit messages to find the suggested replacement. Bad UX.

Re: Git's list of banned C functions

#20

Post by [deleted] » Thu, Mar 04, 2021, 8:57 PM UTC

drfuchs wrote: post #6

It would be nice if the error messages generated would suggest replacement functions that they deem appropriate. I see that I'm not supposed to use gmtime, localtime, ctime, ctime_r, asctime, and asctime_r; but what do they think I should use?

[deleted]