Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Banned C standard library functions in Git source code

github.com

Post reply on HN
Print view View original

281–290 of 329 posts

Re: Banned C standard library functions in Git source code

#281

Post by pjmlp »

stjohnswarts wrote: post #220
zxcvbn4038 wrote: post #215

Earlier quoted context omitted.

There are countless libraries that add higher level string functions and no end to higher level languages. C fills the niche where you want something higher level then assembly but lower level then Perl, ruby, python, etc. Sometimes you want or need to manage your own memory. Arduino is a good contemporary example.

Exactly. C philosophy is to use libraries and not put things like a better string library in the core functions of the language. It keeps the language relatively clean and easy to understand, unlike c++

Bonus points when the libraries are so incompatible among themselves that require extra conversation steps, and then just get dropped 'cause "mind the performance".

Re: Banned C standard library functions in Git source code

#282

Post by MrStonedOne »

wincent wrote: post #237
MrStonedOne wrote: post #202

Earlier quoted context omitted.

Git is open source, it doesn't have its own developers, every developer is "gits own developers". They really should make this more explicit.

They probably don't want the source code to start resembling a Literate Programming example.

I don't think gatekeeping contributors and gatekeeping contributions are the same thing.

Re: Banned C standard library functions in Git source code

#283

Post by bigtrakzapzap »

These shouldn't be used either (_s are available in C11, _l tend to be locale-parameter thread-safety, and _r have sizes/state for bounds/thread-safety):

- strtok -> strtok_r / strtok_s

- asctime -> strtok_r / strtok_s

- gmtime -> gmtime_r / gmtime_s

- localtime -> localtime_r / localtime_s

- ctime -> ctime_r / ctime_s

- dirname -> dirname_r

- basename -> basename_r

- devname -> devname_r

- readdir -> readdir_r

- ttyname -> ttyname_r

- gamma -> gamma_r

- lgamma -> lgamma_r

- lgammaf -> lgammaf_r

- lgammal -> lgammal_r

- atoi -> atoi_l

- atof -> atof_l

- atol -> atol_l

- atoll -> atoll_l

- gets -> gets_s

- scanf -> scanf_l / scanf_s

- fscanf -> fscanf_l / fscanf_s

- sscanf -> sscanf_l / sscanf_s

- tmpfile -> tmpfile_s

- fopen -> fopen_s

- getenv -> getenv_s

- strdup -> strndup

- strcmp -> strncmp

- strlen -> strnlen

- (Multibyte/wide conversion functions without mbstate_t parameter)

- wcslen / wscnlen -> wcsnlen_s

- wcsncasecmp / wcscasecmp_l / wcsncasecmp -> wcsncasecmp_l

- strcasecmp / strcasecmp_l / strncasecmp -> strncasecmp_l

- bzero (use explicit_bzero, in some cases)

- calloc, realloc -> reallocarray (for arrays of non-byte items)

- memmove -> memmove_s

- strncat -> strncat_s

- strncpy -> strncpy_s

- srand / rand -> rand_r

There are others that are platform-specific. Thread-safety, internal mutable state (not thread-safe), and buffer-overflows are the primary concerns that aren't necessarily applicable in all situations.

Re: Banned C standard library functions in Git source code

#284

Post by Mikhail_Edoshin »

crankylinuxuser wrote: post #80
slovenlyrobot wrote: post #56

Earlier quoted context omitted.

These are used /everywhere/ in PostgreSQL, exactly for exception handling. The result isn't bad at all, but indeed, too powerful a tool for 99% of developers

And that's the perfect usage for goto. Error catching and jump to fail block...

It's a different use case. There was a nice article[1] about exceptions in plain C with setjmp and longjmp.

[1] http://www.on-time.com/ddj0011.htm

Re: Banned C standard library functions in Git source code

#285

Post by masklinn »

LukeShu wrote: post #41
alister wrote: post #26

I'm glad to see that setjmp() and longjmp() are still allowed. I'm just kidding by the way. For those C programmers who haven't encountered these before, it is a powerful way to do a "goto" in C. Powerful in the sense that you can jump anywhere, not limited to the same function. If it's used at all these days, it's used for exception handling. More info: https://en.wikipedia.org/wiki/Setjmp.h

The way I see the list is that it isn't meant to be exhaustive, it's meant to be functions that contributors are likely to try to use. A new contributor might not realize that they should be using Git's internal strbuf.h instead of the libc string functions. It's a way to give them feedback on that from their compiler, before they spend more time on the patch and send it to the mailing list.

Also seems mostly for functions which look innocent and are likely to be used but are very easy to misuse.

setjmp/longjmp is not in this category, it might be argued to be easy to misuse but doesn't look innocent and is not likely to be used in normal codebases.

Re: Banned C standard library functions in Git source code

#286

Post by masklinn »

slovenlyrobot wrote: post #56
alister wrote: post #26

I'm glad to see that setjmp() and longjmp() are still allowed. I'm just kidding by the way. For those C programmers who haven't encountered these before, it is a powerful way to do a "goto" in C. Powerful in the sense that you can jump anywhere, not limited to the same function. If it's used at all these days, it's used for exception handling. More info: https://en.wikipedia.org/wiki/Setjmp.h

These are used /everywhere/ in PostgreSQL, exactly for exception handling. The result isn't bad at all, but indeed, too powerful a tool for 99% of developers

Apparently they're also used for error handling in libpng. That's one of the reasons the libspng author started on their project.

Re: Banned C standard library functions in Git source code

#287

Post by masklinn »

Cogito wrote: post #260
peff wrote: post #138

I see a lot of comments to the effect of "shouldn't XYZ also be banned". The answer is that we're not necessarily trying to be exhaustive. The point is to flag common errors before we even hit review, so we add new functions mostly when somebody tries to misuse them. I don't recall anybody trying to abuse longjmp() in Git's codebase yet (and no, that's not a challenge).

Peff, for the people asking in the thread, is there a place where correct alternatives are suggested or demonstrated? I know there are a few different places that talk about how to use git's internal machinery, but not sure if any are specific to these banned functions.

The original commits mentions git's strbuf API[0] and its xsnprintf, a variant of snprintf which asserts that the destination buffer was big enough[1] (rather than just return truncation information).

For other codebases, snprintf is the usual recommendation, and careful straight buffer manipulation (mem*) iff performances are a concern.

[0] https://schacon.github.io/git/technical/api-strbuf.html

[1] https://code.forksand.com/linux/git_git/commit/7b03c89ebd103...

Re: Banned C standard library functions in Git source code

#288

Post by masklinn »

bigtrakzapzap wrote: post #283

These shouldn't be used either (_s are available in C11, _l tend to be locale-parameter thread-safety, and _r have sizes/state for bounds/thread-safety): - strtok -> strtok_r / strtok_s - asctime -> strtok_r / strtok_s - gmtime -> gmtime_r / gmtime_s - localtime -> localtime_r / localtime_s - ctime -> ctime_r / ctime_s - dirname -> dirname_r - basename -> basename_r - devname -> devname_r - readdir -> readdir_r - tty…

Annex K was never very popular, is still very easy to misuse[0] and has pretty much been deprecated and slated for removal.

[0] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm#mi...

Re: Banned C standard library functions in Git source code

#289

Post by masklinn »

LukeShu wrote: post #36
saagarjha wrote: post #34

What does Git use instead for copying strings? snprintf? Edit: also interesting is a search for alloca: https://github.com/git/git/search?utf8=&q=alloca&type=

Git has an internal "strbuf" library https://github.com/git/git/blob/master/strbuf.h

Alternatively it also uses snprintf, though as a small internal variant (xsnprintf) which literally kills the program if the destination buffer is too small: https://github.com/git/git/blob/master/wrapper.c#L636-L650

Re: Banned C standard library functions in Git source code

#290

Post by dezgeg »

begriffs wrote: post #247
dezgeg wrote: post #213

Earlier quoted context omitted.

If your buffer isn't a NUL-terminated, then don't call a function that is only defined for NUL-terminated buffers. It's as simple as that. I'm baffled by how some people claim strlcpy() is 'broken' or 'not safe' because it doesn't handle non-NUL-terminated inputs; the exact same thing applies to just about any function in the C standard library that takes strings as input. Are functions like strchr(), fopen(), printf…

> If your buffer isn't a NUL-terminated, then don't call a function that is only defined for NUL-terminated buffers. It's not that people want to pass strncpy source buffers that lack NUL termination, it's that strncpy in certain situations will not NUL terminate its results. https://begriffs.com/posts/2019-01-19-inside-c-standard-lib.... > some people claim strlcpy() is 'broken' Speaking of strlcpy, it thankfully do…

Yes, strncpy() not NUL-terminating it's output is nasty and well-known. But the comment I was responding to was claiming strlcpy() being unsafe.
Post reply on HN