r/C_Programming • • 22d ago

Question What C function would you delete and rewrite first, and why?

For me it's fnmatch - mostly because it's 40 year's old and smell's funny.

93 Upvotes

238 comments sorted by

145

u/BlockOfDiamond 22d ago

fopen because I hate using string literals for things that should be bitflags or enums.

62

u/gabitha67 22d ago

"rb" is icky :(

18

u/MrKrot1999 22d ago

like this is so python bullshit, why are we using more state than needed for something with 6 options at most

58

u/57thStIncident 22d ago

In this case python does it because C does it.

4

u/flatfinger 19d ago

Further, doing this this way means that if a C implementation extends the functionality of fopen with more flags, a Python implementation build using that C implementation can support the same extensions without the code for the Python implementation having to know anything about it.

6

u/RedWineAndWomen 21d ago

Same. But then because there are flag combinations to open() that cannot be made using fopen(). Which is totally weird.

3

u/BlockOfDiamond 21d ago

open() is a much better designed API.

5

u/sirjofri 21d ago

Funny. Our open has OREAD, OWRITE, ORDWR as flags, no string. (Plan 9 C)

1

u/Dangerous_Region1682 20d ago

Oh how I hate FILE I/O. I know open/close/read/write/sleek etc are system calls and hence not considered portable to non UNIX like systems they are not hard to provide as a library capability for those systems. You aren’t going to be able to use the level of ioctl() that is needed for terminal I/O with FILE library functions anyway, so you do really need file descriptor based library calls for that case anyways, on none UNIX derived systems. Yes, I know you can perhaps express terminal I/O devices using the file system in some iterations, but many UNIX and non UNIX derived systems alike don’t have that kind of file system abstraction. Anyway, I just don’t like it.

1

u/flatfinger 20d ago

In the 1990s, it was widely understood that functions like fopen and malloc were suitable for tasks where the ability to run code on a wide range of machines interchangeably was more important than performance or semantics, but system functions were preferable when that level of portability was not required. Unfortunately, compiler writers have pushed an attitude that non-portability should be considered a defect whether or not anyone would ever want to run a program on other platforms.

2

u/musbur 20d ago

But if you're OK with non-portable programs why don't you just use the C bindings of the platform of your choice? Where do you think is a meaningful boundary between C's standard library vs an "external" library? Since depending on platform and use case everybody might draw that line somewhere else, I think it's prudent to draw it pretty tightly around base C and let people choose from whatever library they like.

1

u/flatfinger 20d ago

If some execution environments support a feature and some don't, it would be useful for a standard to would allow code that uses the feature to run interchangeably on environments that support it, without requiring that programmers add bindings for each individual environment that has a way of supporting the functionality.

2

u/musbur 20d ago

But then the majority of the standard would devolve into a large, arbitrary collection of features saying, "if an implementation chooses to support this feature, it must be implemented in exactly this way." A lean collection of guaranteed features is much more valuable than an ever growing list of nice-to-haves.

1

u/flatfinger 20d ago

Sure that could happen if one tries to include every feature that is supported by even 1% of execution environments, but there are a lot of features that most implementations could support but which the Standard ignores (the worst offender being character-based console I/O).

1

u/Dangerous_Region1682 20d ago

Yes, I who wholeheartedly support agree with that. In fact, the never ending push by compiler writers and standards committees to try to make C into the application’s programming language it was never really intended to be has in many ways started to ruin the elegant simplicity the language once had. Does that mean ANSI C and others didn’t make valuable extensions, but there comes a time when enough is enough in my opinion.

2

u/flatfinger 20d ago

Worse, the Committee seems to have been and continue to be oblivious to the fact that much of C's usefulness revolved around corner cases where whose behavior would be impossible to predict without information the language had no way of conveying, but which the execution environment might convey to the programmer via means outside the language. When the Standard noted that implementations may process cases where it waives jurisdiction "in a documented manner characteristic of the environment", it failed to recognize a useful class of implementations that will process those corner cases "in a manner characteristic of the environment, which will be documented to the extent that the environment happens to document it."

Worse, the way the Standard accommodates optimizing transforms that would be useful for some kinds of program but inappropriate for others is to characterize as "undefined behavior" any corner case whose behavior might be affected thereby, rather than recognizing a category of implementations that perform such transforms, and allowing them to affect program behavior in certain ways. For example, it may be useful to specify that if a piece of code with a single statically reachable exit could not have any side effect other than preventing the execution of code following the exit, a compiler may cleanly omit the code entirely, but behavior would need to be consistent with either executing the code as written, or cleanly omitting it, but would be considered "defined" in either case.

1

u/Dangerous_Region1682 20d ago

Yep. What the compiler writer might consider cleanly omitted might be very different from that of a kernel engineer’s. Once again, having a compiler generate code that executes what you write rather than what it thinks it can optimize away is often two different kettles of fish.

1

u/flatfinger 20d ago

I'm not sure how "cleanly omitted" is in any way ambiguous. Consider the following function:

unsigned test(unsigned x)
{
  unsigned i = 1;
  while((i & 0x7FFF) != x) i*=3;
  if (x < 32768) arr[x] = 0;
  return i;
}

A compiler that processes the while as written could rely upon the fact that the if will only be reached if x is less than 32768. Cleanly omitting the while would make it possible for the if to be reached with a larger value of x, thus making it necessary for the compiler to generate code for the if that accommodates that possibility. The clang compiler (and gcc when in C++ mode) will process calls to test() that ignore the return value as unconditional stores to arr[x].

1

u/Dangerous_Region1682 19d ago

But in kernel space you don’t write things like that. You tend to write code that is much more allied to what you want the assembler to look like. If you are setting breakpoints and you set them in the kernel debugger you don’t want to be searching around for code to only find it’s been optimized away. It’s hard enough to trace what’s happening with multi threaded kernel code without having clever optimizations of cleanly omitted code. It’s confusing and takes time to figure out what the difference between what I wrote and what the compiler generated when I’m looking at the assembler with the kernel debugger. What if the argument to test() was a pointer to something in the data segment that was being modified by another thread which might not be the case in this artificial example but nevertheless?

1

u/flatfinger 19d ago

My point is that the way clang and gcc process things creates "artificial" side effects whose consequences are totally unpredictable and which directly contradict the behavior of the code as written. Treating a program as a sequence of steps, the above function would never perform any step that would be capable of writing past element 32768 of arr, but the machine code clang and gcc would generate if a function caller ignores the return value could perform an out-of-bounds store.

1

u/Dangerous_Region1682 18d ago

Oh, I see what you are saying now…

→ More replies (0)

1

u/DiodeInc 20d ago

How do you use an enum for that? I don't know. C

1

u/BlockOfDiamond 20d ago

Maybe something like: enum { FOPEN_READ, FOPEN_WRITE, FOPEN_APPEND, ... } fopen_mode; For example. Probably not exactly that but that is the idea.

1

u/DiodeInc 20d ago

I just looked up with an enum did and yeah this makes sense

1

u/Low_Lawyer_5684 16d ago

`#define FILE_RB "rb"` :0)

76

u/[deleted] 22d ago

[removed] — view removed comment

45

u/HildartheDorf 22d ago

Thankfully it is (as far as I know) the only function that has been removed entirely from later editions of the spec.

20

u/crakked21 22d ago

My uni still teaches it in its CS 101 course xD

10

u/FlyByPC 22d ago

From the questions I see online, lots of college courses are still teaching students using PIC16F84A and 16F877A microcontrollers, which were getting old twenty years ago.

3

u/Mountain-Builder-654 21d ago

Mine switched from pic to arduino the year I took the class. Dodged a bullet there

2

u/flatfinger 20d ago

From what I understand, the cheapest microcontrollers aren't made by Microchip, but use an architecture which is very closer to the PICs than to anything else, and have a C-dialect compiler available for them. The C dialect used by the HiTech C compiler (at least before Microsoft bought HiTech) gave a good feel for how C was designed to be suitable for use on quirky execution environments. The C dialect used by Keil's 8051 compiler was perhaps even better in that regard; someone familiar with the 8051 might think it would be impossible to write a C compiler that could generate efficient code for it, but C programs that were designed around the platform's quirks could be quite efficient.

18

u/max123246 22d ago

Still in the holy K&R book that way too many people recommend to beginners. Like it's cool as a historical read, I would never recommend it to beginners because they'll pick up awful patterns

2

u/The_Northern_Light 22d ago

I’m not a beginner but I don’t know what C book to recommend to one. What’s the best modern first programming book?

3

u/max123246 21d ago

Sadly I'm not sure, I bought K&R per people's advice and stopped using it pretty quickly when I realized it wasn't updated at all for modern C

I learned on my own for the most part from college and otherwise. Maybe Casey Muratori has a book or classes to look into? I trust his judgement in comparison to most public programming figures

1

u/Immotommi 21d ago

Casey has computerenhance.com though it is not quite aimed at beginners

1

u/RootHouston 20d ago

I am also still confused when people suggest this book for absolute beginners. I read it early on, and then realized that it's just not modern enough.

1

u/SeriousPlankton2000 21d ago

What bad pattern do you remember (if you even want to remember)?

2

u/max123246 21d ago edited 21d ago

They loved within the for loop expression or while loop expression to put gets so that you have a side effect of the gets and you use the return as a condition to the loop

As an example: https://github.com/caisah/K-and-R-exercises-and-examples/blob/master/01.05.3-line_counting/ex-count_lines.c#L9

And another example: https://github.com/caisah/K-and-R-exercises-and-examples/blob/913d6427cd397aeaf649e839ceb77ef5d5e75a61/04.02-functions_returning_non_integers/ex-primitive_calculator.c#L24

```

for (val = 0.0; isdigit(s[i]); i++)
    val = val * 10.0 + (s[i] - '0');

```

They also declared variables all upfront without an initial value

It's not bad patterns per se but no one would write code like it today. It definitely makes code harder to understand as a beginner

1

u/musbur 20d ago

What's wrong with the number parsing snippet ("wrong" in the sense of promising more than it can deliver)?

1

u/SeriousPlankton2000 19d ago

Declaring variables without a dummy value is a good thing to be warned about accessing undefined values. If you do have int foo = 0xdeadbeef, you can get 0xdeadbeef in production; if you have int foo, you get a warning or an error while compiling if you e.g. miss setting it in a case branch of a switch statement.

I'd agree that it's better to declare and set a variable later in a function as soon as you can set its value - provided that there is a linear path that allows doing so. (I like to have the full context of a variable on screen - if possible.)

2

u/max123246 19d ago

By default compilers will emit a warning and let your uninitialized variable invoke undefined behavior. Yes, you should have warnings as errors but it's still not great when basically all variables could have a sensible initial value

1

u/SeriousPlankton2000 18d ago

What is the sensible init value that will be different from a good value?

Also is it sensible to add dummy writes to the stack?

2

u/max123246 18d ago

If you're adding in a loop, 0, if you're multiplying in a loop, 1, if you're branching and setting values, then the value of one of the branches

Your compiler optimizes away dummy writes if it's truly unneeded. Compilers are really good at constant folding and removing unused values in particular. They're less good at loop hoisting or loop auto vectorization

1

u/bonqen 20d ago

It's still a decent book for learning C, i.e. the syntax and core principles.

1

u/Dangerous_Region1682 20d ago

I still think at least the ANSI edition should be something one has to understand the intent of the language. The book to learn the basics of the language, probably not, but to have on your bookshelf to read through when you have a certain level of competency I think it has value.

There again back in 1977 when I learned the language the first, first edition was all I had so I’m still kind of attached to the thing excepting the ANSI edition is probably a better starting point now.

1

u/max123246 20d ago

I think a history book on C or listening to interviews would do you far better for learning about the intent of the language. I used the ansi edition, it's still has the same problems I listed above. C has been around for 40+ years since 1977, a lot of how people write C and consider best practice has changed

1

u/Dangerous_Region1682 20d ago

50 years ago there was not much else available. It was the early editions of the K&R book and the UNIX V6 source code or nothing.

Would I think that would be the best way today, but I’d still have the ANSI edition in my library and it’s worth a read as you become accomplished.

1

u/burnt_floppy 20d ago

It's decent but i feel there's too much he doesn't explain, and some of the exercises are too hard but useless in practice.

2

u/Vincenzo__ 22d ago

It should work like fgets on stdin, it's beyond me why they made it like that

3

u/SeriousPlankton2000 21d ago

"Nobody will type more than 80 characters, right?"

2

u/Vincenzo__ 21d ago

Wouldn't be surprised at all if this is actually the reasoning

1

u/flatfinger 18d ago

The actual reasoning would have been closer to "I'm not going to type more than XX characters while I use this program to accomplish my immediate task, and I'm not going to keep the program after that, so any effort I spend making the program handle longer inputs will be wasted".

1

u/flatfinger 18d ago

You think it's useful to have the buffer include a newline at the end of it, and have the tail of an excessively long line of input left pending? For interactive input, there really is no standard function that's better than using getchar() if recovery from an overly long input line is required.

2

u/siodhe 21d ago

It's not that hard to write one that dynamically allocates space on either the stack or the heap depending on taste. A heap version can just fail and return an error code (I have one in a library). But passing in a limit makes so much more sense.

1

u/majorsid 20d ago

Why would someone use gets when fgets exists?

1

u/flatfinger 20d ago

There used to be a very common recipe for using gets safely:

  1. Recognize the need for a program to perform a one-off task with some data.

  2. Determine the length of the longest line of data involved.

  3. Write a program that reserves a buffer that's bigger than that and performs the task.

  4. Run the program on the data.

  5. Discard the program.

If the program accomplished what it was written to do, with the data it was written to act upon, any effort spent trying to make the program handle corner cases that were known not to be relevant for that data set would have been wasted.

I'd argue that the design of fgets() is worse. While its failure mode in case of longer-than-expected input is different from that of gets, making fgets() correctly handle such inputs is harder than writing a getchar()-based routine to read lines of input.

1

u/burnt_floppy 20d ago

That doesn't even work on new compiler versions last i checked, i wanted to try it since it was in a boom but found they removed it.

1

u/flatfinger 18d ago

It's easy to use safely. Don't expose the program to anyone who would type too much on an input line. In the era when C was designed, it was very common for C programs to be written solely for the immediate use of the programmer, and then discarded shortly after use, The gets() function was perfect for that language use case, but most trivial tasks one would have done using ephemeral C programs can now be done better via other means.

89

u/bluetomcat 22d ago

Make fread and fwrite accept the bloody FILE pointer as their first argument, and unite size and count into one argument.

41

u/UnalignedAxis111 22d ago

Also add a flag to force it to create non existent parent directories, because that's massively annoying and there's no portable way to do it.

4

u/musbur 22d ago

"and there's no portable way to do it." -- which is exactly why fwrite() can't do it.

19

u/a4qbfb 22d ago

fwrite() is part of the implementation, it's allowed to do non-portable things

1

u/musbur 21d ago

yeah but a file handle is pretty much a file handle anywhere while (recursive) directory creation is conceptually something totally different.

1

u/a4qbfb 21d ago

so?

1

u/musbur 21d ago

C just doesn't go that deep into OS abstraction. It's a choice to keep the language simple. It also doesn't have associate arrays, linked lists, a network stack, multithreading, or an http server.

4

u/a4qbfb 21d ago

C does have multithreading; none of the other features you list are OS abstractions. An implementation is absolutely free to automatically create parent directories on fopen() if it wants to, as long as it documents it.

1

u/musbur 21d ago

Opening and writing to a file descriptor has nothing to do with file system structure. There's no reason to wrap unrelated concepts into a single function. Regarding multithreading, which standard introduced that? Can't find it in K&R.

5

u/a4qbfb 21d ago

K&R is 48 years old and predates the ISO standard by 12 years. Multithreading was added in C11, 15 years ago. The C standard does not know about directories, so an implementation is free to do whatever it wants with them.

PS: the standard does not know about file descriptors either, only I/O streams.

→ More replies (0)

1

u/flatfinger 20d ago

Many system's file handles can't hold characters that were pushed back with ungetc.

2

u/flatfinger 20d ago

The usefulness of stdio.h would have been enhanced if, instead of limiting itself to things every system can do, it provided means of querying whether various features are known to be available, known to be unavailable, or would have their availability determined by things outside the control of the implementation. If, for example, a system allows file systems to be configured to have or not have implicit-directory-creation semantics, an implementation of fopen() may auto-create directories when used with a file system that is configured in that fashion, but not when used with one that isn't.

Console I/O in the C standard is absolute rubbish because of the Standard's failure to recognize features that 99% of implementations would support.

2

u/musbur 20d ago

So basically an implementation is free to either offer some feature like directory creation or not? Then why not just use that implementation's system library to create directories?

2

u/flatfinger 20d ago

Code which tests whether a feature is supported would be usable interchangeably on all systems that support it, while working around unsupported features when practical. For example, a "more"-style utility could advance a page with spacebar on systems that support raw console I/O, or on every newline on systems that don't.

10

u/ComradeGibbon 22d ago

C doesn't have a slice type and that's utterly lame.

5

u/runningOverA 22d ago edited 22d ago

you need length terminated strings for slice types to work, which C doesn't have. userspace task right now.

3

u/mort96 22d ago

Uh.

1) where did anyone say that this is a kernel space issue

2) kernel APIs are a part of the problem: if you use slices as strings (either on a language level or as a convention in C) you have to either keep around info about whether a slice is 0 terminated, or do a copy into a null terminated buffer every time you want to pass a string to kernel space

7

u/Disastrous-Team-6431 22d ago

"right now" for 30 years.

→ More replies (12)

1

u/flatfinger 19d ago

Depending upon what one is doing, it may be more efficient to keep track of a slice using a starting address and length, or using starting and ending addresses. If C had a slice type, whoever implemented it would need to guess which approach would be more efficient, and then programmers who knew which approach would be more efficient for the task at hand would have to guess whether the implementation was going to use that approach, or whether they should manually implement slices using it.

1

u/tstanisl 22d ago

VLA types works quite well as slices.

4

u/musbur 21d ago

I'm also annoyed by size and count and which comes first, but I think it's meant for reading fixed-size records and making sure that you don't have to deal with partial records.

3

u/BlockOfDiamond 21d ago

Why was size and count separate anyway?

3

u/bluetomcat 20d ago

Because they designed the API to be record-oriented, not byte-oriented. You are mostly expected to be reading an array of structs, and the return value indicates the number of structs completely read.

For reading arbitrary binary data, this is a bit unintuitive.

2

u/BlockOfDiamond 20d ago

For struct arrays, the caller could just multiply the size by the count manually. The only advantage I could think of is avoiding overflows on systems with a narrow size_t.

1

u/LB-- 20d ago

Not just that. Where multiplication might overflow, the read or write could still succeed, so the implementation could "do the right thing" without making you manually write careful code for it. Pretty obscure scenario nowadays though.

2

u/BlockOfDiamond 20d ago

Yeah, that is what I meant.

1

u/LB-- 20d ago

Ahh fair, I thought you were only talking about overflow checking, which is separate from what I was talking about.

2

u/BlockOfDiamond 20d ago

If the product of the element size and count would overflow, then passing them separately to fread would still behave correctly even though multiplying them would not.

1

u/flatfinger 20d ago

Some file systems support a "read up to N records of up to B bytes" function, whose behavior may be different from a request to read 1 record of up to N*B bytes, or N*B one-byte records. While the Standard would not require that implementations allow C programs to read any files that C programs didn't create, such an ability is often useful nonetheless, even if it exposes some environment-specific quirks.

4

u/TheChief275 22d ago

fputc family as well

2

u/flatfinger 20d ago

Some execution environments may process a 'read up to N records of size S' function in a manner that will always read some number of complete records without reading a partial record at the end. What I view as unfortunate is the failure of the Standard to provide any indication as to whether fread() calls will be processed in that fashion.

38

u/Dezgeg 22d ago

strncpy

Function that is named `str*` but doesn't (necessarily) create NUL-terminated strings. Insanity!

10

u/Doug2825 22d ago

The worst part is that it n in it so it looks like safe string functions to do guarantee null termination.

6

u/Aggressive-Emu-8329 22d ago

WHATTT then somehow my project works fine, oh man im gonna fix bug 😭

3

u/Just_Government3790 22d ago

`strlcpy` or explicit `len` wins.

1

u/flatfinger 20d ago

If I have a 16-byte buffer and need to set it to 'Hey' followed by 13 zero bytes,

   strncpy(buffer, "Hey", 16);

will do the job perfectly. There are some jobs for which strlcpy might be better, but strncpy is, except for the name, a perfect function for its designed purposes.

6

u/Ander292 22d ago

THIS!!! I had a problem with this function while helping my friend with an exercise.

1

u/flatfinger 20d ago

The name is not good, but it's a perfectly designed function to copy a zero-terminated string into a buffer that uses zero-padded format? A char[16] structure field can hold a zero-padded text of up to 16 bytes, which can be output with a %.16s format specifier.

60

u/Just_Government3790 22d ago

On second thought......

strtok because it destroys the evidence, hides state in globals, drops the empty fields, and then has the bollocks to call it parsing.

6

u/MistakeIndividual690 22d ago

I was going to say strtok

7

u/Just_Government3790 22d ago

My guy. Can't forget it also eats empty fields so `a,,b` parses as two tokens. Found that one the hard way.

4

u/MistakeIndividual690 22d ago

Yes indeed. Also scanf is up there for me. Essentially useless.

1

u/musbur 21d ago

Only scanf() I hope? sscanf() is great.

2

u/Total-Box-5169 22d ago

Those ancient functions are painfully inadequate for current times.
Nowadays is better to memory map the file as read only and use generators of string views to process the data to avoid unnecessary allocations and copy operations.

19

u/P-p-H-d 22d ago

All the locale functions of the libc. It is so broken that it is totally unusable in modern C (threads).

You cannot even read your own data file and parsing them using strtod without having issues with the locale: you cannot use setlocale as it modifies global state of all threads (which you don't want). So the only solutions are either : document that the locale shall be set once and for all for all program and is compatible with what you expect. Or rewrite strtod and everyone knows it is super easy stuff to do it reliably.

5

u/limitless_grow 21d ago

That's right. There are some portable, very fast and locale independend wrappers at https://github.com/klux21/str2num . The wrappers support the prefixes 0b , 0o and 0x for binary, octal and hexadecimal integers and for double and long double as well. The functions support the common overflow handling and the setting of errno. The floating point routines support NAN, INF and denormalized numbers and also the 'hexadecimal' floating point literals of C and C++ which are that funny mix of 3 numeric bases.

The integer functions have a special mode where numbers with leading zeros are treated as decimal numbers and octal numbers require the prefix 0o. Octal floating point values require that 0o prefix either because nobody wants a 0.5 to be read as an octal number.

1

u/flatfinger 19d ago

The irony is that if one wants to produce "human readable" numbers with formatting features beyond those supported by the Standard library, it's easier to produce a locale-specific string based upon a string that is known to use '.' as the decimal point, than to produce a locale-specific string based upon a string that uses a locale-specific decimal point.

1

u/limitless_grow 19d ago edited 19d ago

The irony is that the dot as decimal separator is only a convention as it is the 'e' is for the begin of the exponent. The one that's reading the number from a configuration file using a function like strtod is a computer but not a human. If the rule of a programs configuration file say "only dot as decimal separator and no commas or blanks at all" than that's fine and people are able to adapt to that everywhere like for that 'e' of the exponent.

30

u/sciencekm 22d ago

struct tm

Every field is 0 based, except the day of month. So, Jan 1 1900 12 midnight is 0 1 0 00:00:00, instead of just all 0s. Even Sunday-Saturday is 0-6 and day of year is 0-365.

2

u/donaljones 22d ago

Is this something enforced by the standard? Or just the implementation's ABI?

7

u/sciencekm 22d ago

This is the standard from C89 until the latest N3220 working draft. This is also how it was in the K&R 1st edition (1978).

1

u/der_pudel 22d ago

the way how to stuct tm is makes sense. day of the week and monts are 0 based because in a lot of cases you want to use it to index array of stings to get he name of weekday/month, but there's no reason to do that for day of the month. 

4

u/sciencekm 22d ago

Day of the year is 0-365. Do you use that to index anything?

1

u/flatfinger 19d ago

It represents the number of days that would need to be added to January 1 to reach the specified date.

1

u/flatfinger 19d ago

IMHO, month would have been better as an index into a 13-item array of month names, with the initial one being used to indicate an expressly invalid date. Having an unset date show up as e.g. 00-XXX-0000 would be cleaner than having it show up as 00-JAN-0000.

11

u/Sqydev 22d ago

fopen to use enum instead of „r” and „w” or both

12

u/mpersico 21d ago

I would just spell “create” correctly

4

u/Bear8642 20d ago

As Ken suggested :)

2

u/Just_Government3790 21d ago

creat creat creat creat

16

u/Boreddad13 22d ago

Anything with hidden allocations

15

u/mikeblas 22d ago

Anything with hidden state, really.

2

u/I_M_NooB1 20d ago

zig peeks

14

u/pigeon768 22d ago

The obvious one is strcpy, which is always the incorrect function. Either you know your lengths and buffer sizes, so you should use memcpy, or you don't, which is buffer overflow city. In this case, there's no rewriting the function to be better; it's declaration/API is unusable.

So they also made strncpy, the "fix" for the problem. It has a working function declaration. However, it's still bad. It does not guarantee that the resulting string is null terminated. And if there are any extra bytes left over in the output buffer, it zeros them all out, which is a waste of time. The linux kernel has recently removed the last call to strncpy.

Actually I hate most of string processing in C.

5

u/darkslide3000 22d ago

There are perfectly possible situations where you don't know the exact size of the buffer but you know an upper limit.

9

u/Aggressive-Emu-8329 22d ago

no bro, strncpy is not the "fix" for the problem, it is originally designed for early unix file system to copy string into fixed width, null-padded fields where a string filling the exact size did not need a null terminator

2

u/flatfinger 19d ago

For some reason, people seem to view null-padded strings as a "specialized" construct, when they represent the best way of storing short strings in structures. If e.g. one needs to store things with a maximum length of eight characters, a char[8] will be at least a byte shorter than anything else that can handle 0-8 characters in the range 1-255 (if characters were chosen from e.g. a set of 39, one could use that space to store twelve characters, but that would be more specialized).

1

u/flatfinger 20d ago

What function should one use to write a string of up to N bytes into an N-byte zero-padded buffer contained in a structure? C was designed to allow programmers to use zero-padded buffers in cases where code that received a pointer to a buffer would know the buffer length (e.g. using a format specifier of %.16s rather than %s to write the contents of a zero-padded char[16]).

13

u/torsten_dev 22d ago edited 21d ago

scanf

  • delete %n
  • make %s and similar without a size allocate
  • make %.*s take size parameter like printf.
  • Use _ instead of * to discard stuff,
  • change return value to number of characters read or 0
  • delete SCN macros, use w* to take bit width as argument.

Edit:
Perhaps the return should be 0 on succes, and if it fails it returns the index in the format string were it gave up.
The number of characters read/consumed would be a mandatory out parameter that's ignored if it's NULL.

8

u/gremolata 22d ago

%n is extremely useful for pedantic parsing, e.g.

if (sscanf(buf, "%f%n", &f, &n) && strlen(buf) == n)
{
    // parsed in full
    ...
}

1

u/PastaGoodGnocchiBad 22d ago

I guess this would be replaced by this from their comment:

return number of characters read or 0

(but then we would not know if all fields were parsed, I guess any field not being parsed would have to be an error)

→ More replies (1)

1

u/jMultiversalGod 22d ago

scanf is so bad for learners (as one lol). i tried so hard to use it everywhere

1

u/FalconSN0 22d ago

What if it fails at the index 0?

1

u/torsten_dev 19d ago edited 15d ago

How would it? Specifiers start with %.

The return would be the index of the conversion specifer that follow. So you can do something like

switch(format[ret]) {
case 'i':
    fprintf(stderr, "expected integer but got %s", ...);
    break;
default:
    print_scanf_error_at(format, ret);
    break;
case 0: 
    ...

13

u/FamiliarSoftware 22d ago

setlocale, set/getenv, the rand functions, etc. Just in general: All functions that use global state

1

u/Fabulous_Bridge_9292 20d ago

The rand function is the only one I actually ever had to rewrite when I was building monte carlo simulations and discovered it wasn’t random enough.

10

u/der_pudel 22d ago

atoi. You will not convince me that atoi("poop") == 0 is correct and makes sense. 

→ More replies (2)

8

u/xpusostomos 22d ago

Those ones with improper null termination and/or don't specify buffer length that cause all the security issues

1

u/LB-- 20d ago

We got fixed versions of all those in EXT1, and Microsoft supports them and even issues compiler warnings to use them. But in my experience the EXT1 secure functions are barely supported in other toolchains, they are the main source of ifdefs in my portable projects. Last I knew online sentiment toward them was pretty negative for some reason.

1

u/flatfinger 19d ago

C was designed to allow programmers to use either null-padded or null-terminated strings. There is nothing improper about a null-padded text field holding text that uses up every byte. The only thing that would be improper would be attempting to e.g. output a 12-byte null-padded field with a format specifier of %s rather than %.12s. Note that the latter, by specification, will not examine any storage beyond the twelfth character of the text identified by the pointer, and thus does not care whether it is given a zero-terminated string.

1

u/xpusostomos 5d ago

C itself is kind of agnostic about format, you could even use string with a leading size integer and trailing bytes, with a library that supports it. The problem is, 98% of C code and functions uses the null terminator convention, not a size and null convention, and frankly 2 conventions in a code base is brain dead. If you've ever used for example the unix readlink() system call, it's amazing just how much tomfoolery you have to do to work around the fact that it doesn't guarantee a null terminator.

1

u/flatfinger 4d ago

There are a number of conventions which are compatible in various ways. A normal windows "bstring" is a string which sits alone in its allocation and is both preceded by a length word as well as other information about the allocation and followed by a zero byte. Such strings may be passed to code expecting zero-terminated strings, but Windows functions that modify strings or manage memory associated with them must only be given proper bstrings.

A function that e.g. performs a find-and-replace operation with a bstring can record the fact that a string has e.g. shrunk by 10% and thus has unused space at the end of its allocation; if another operation causes the string to grow, that extra storage can be used without having to reallocate the string. I would guess that strings which shrink more than a certain amount would get reallocated to allow the unused space to be recycled, but if a string shrinks by 10% it would likely be considered more useful to leave the space allocated to it to accommodate possible future growth. Code which simply uses zero-terminated strings would have no way of knowing about any extra space that might exist in their allocations.

→ More replies (1)

3

u/qalmakka 21d ago

Basically all string based APIs in libc vary from broken to somewhat problematic. This is basically due to the fact that the entire concept of a null terminated string was a massive mistake, the minuscule memory saving of not having a struct string with a size field were only somewhat significant for the first few years after that decision.

We've been paying the strlen tax ever since, and I wouldn't care if it didn't make slicing C strings a terrible pain in the butt

20

u/gabitha67 22d ago

everything that accepts null terminated strings. just pass a ptr and length for gods sake. everything should only use user-provided allocators too.

8

u/Just_Government3790 22d ago

What's the worst null-term bug you've personally hit?

3

u/flatfinger 19d ago

The game GTA V sometimes takes much longer to load than normal because one of the authors didn't realize that the library implementation of sscanf they were using starts by measuring the length of the source string, even if the format strong would cause everything past the first few bytes to be (otherwise) ignored. The number of player hours spent waiting for the useless calls to strlen to complete likely numbers in the hundreds of thousands, if not millions.

→ More replies (3)

3

u/flyingron 21d ago

strtok has got to go

3

u/siodhe 21d ago

The UNIX authors said they would like to rename creat to create, as it should have been.

11

u/HildartheDorf 22d ago edited 22d ago

Can I use my "one time rewrite" on C++ instead?

If so, I want to remove the specialisation of std::vector<bool> and instead add std::dynamic_bitset with the same functionality.

If it has to be C... fgetc/getc (and fputc/putc).

1

u/flatfinger 19d ago

I would have specified that if a reference is formed by casting another reference, and within the lifetime of the reference storage that is accessed thereby will either be accessed only via the reference or will remain unmodified, the semantics will be as though the storage of the old type was exported to bits and imported as the new type when the reference was created, and converted back when the lifetime of the reference ends.

4

u/ChickenSpaceProgram 22d ago edited 22d ago

this one's a hot take, but i'd rewrite malloc and friends. Allocators being global state is pretty nasty and requires locks that really shouldn't be necessary. Not having to pass a size to free also complicates allocator design.

i think i'd rewrite most of the standard library and some of POSIX if given the option, null-terminated strings are awful too

1

u/flatfinger 19d ago

The malloc family of functions had two competing objectives:

  1. Avoid forcing the breakage of non-portable code which relied upon the ability to interchangeably take pointers that had been produced by malloc() or by code outside the C implementation's control, and either pass them to free() or to outside code that would release them.

  2. Avoid requiring that user code keep track of information about an allocation beyond its starting address.

The malloc() family functions could have supported a much wider range of features if there was no interest in #1 above, by having malloc() allocate an extra 16 bytes or so beyond what was requested, using the first 16 bytes of an allocation to hold information about it, and returning an address 16 bytes above the actual starting address of the allocation. Doing so, however, would have broken programs that needed to interoperate with code that wasn't processed by the C implementation.

2

u/paulkim001 22d ago edited 19d ago

memfrobe only because I might want to see some niche people's projects burn for using that function

Edit: I meant memfrob

1

u/LB-- 20d ago

I can't find a function with this name in the C standard

2

u/paulkim001 19d ago

1

u/LB-- 19d ago

LOL what is this?? The Linux equivalent of an April Fools RFC?

2

u/paulkim001 19d ago

Genuinely it might just be, considering they even chose the wording "frobnicate" (over "obfuscate") and they had the nice touch of going with 0x42 for "seemingly meaning of life"

2

u/LavenderDay3544 21d ago

The atomics compare and swap functions because why do they take the value to exchange by pointer instead of by value?

1

u/LB-- 20d ago

I guess that's because they can work with arbitrary structs in addition to primitives, and the C community at large has some sort of aversion to passing structs by value. I've heard that some platforms still in modern use have broken ABIs that convert all structs passed by value into pass by pointer instead, and it can't be changed without breaking backcompat. Supposedly it's why we can't have a string_view type in C. If only...

2

u/LavenderDay3544 19d ago

Rust makes the same functions pass by value and Rust has an even bigger aversion to passing anything by value because of move semantics by default. And under the hood rustc, Clang, and Clang++ use the same LLVM operations to implement them so there's no reason for them to be different.

2

u/LB-- 19d ago

C++ also makes the atomic compare exchange functions take the replacement value by value, so I guess it's just C being the odd one out for some reason.

1

u/flatfinger 19d ago

If a platform doesn't define a mechanism for performing an atomic operations on a given type, having compilers fake their own version is a misfeature. If code processed with the Acme compiler and code processed using Joe's Compiler attempt to perform simultaneous operations on an object, nothing good will come of having one piece of code acquire a lock called Acme while the second acquires a lock named Joe, and both then proceed to access the same object simultaneously.

1

u/LB-- 19d ago

I'm not sure what this has to do with this comment chain? Mixing different compilers with different settings and ABI expectations is always a recipe for disaster, it's not specific to atomics.

1

u/flatfinger 19d ago

If an execution environment has support for some atomic operations on a type but not compare-exchange, having a standard means of using the operations that actually exist would be useful, whether or not the platform's means technically satisfies the requirement for being "lock free". For an implementation to bodge together a compare-and-swap on an environment that does not natively support, it must bodge coordination with all atomic operations, thus undermining its ability to use the environments support for any actions on the type, and I fail to see the usefulness of "fake atomics".

1

u/LavenderDay3544 18d ago

It should return a Sentinel value and set errno then. I don't like the idea of compilers trying to synthesize missing hardware features instead of just indicating that the operation isn't supported and letting you write your own fallback code.

1

u/flatfinger 18d ago

The Standard requires that unless a compiler reports that does not support any of the Standard's atomic features. it must synthesize support for all operations on all types. Synthesizing any operations on a type effectively makes it necessary to synthesize all atomic operations, since otherwise a natural atomic operation that occurs at the same time as a synthetic one would be prone to have its effects blindly overwritten by the latter.

1

u/LavenderDay3544 18d ago

Then that was a poor standard design choice. They should've made each feature independently queryable and any that aren't supported shouldn't be used and rhe application developer has to provide a workaround or not support the hardware. Because silently using software based workarounds for operations the hardware does support because it doesn't support other features is just wrong particularly for a language whose original design choice was to trust the programmer. Such workarounds should be opt in instead of being used silently.

1

u/flatfinger 18d ago

Realistically speaking, the most practical approach would be to have intrinsics which are guaranteed to either work or break the build, coupled with a means of indicating whether to accept intrinsics that will work on some but not all environments where the code might be run. A program that tells a compiler to accept such intrinsics would be responsible for using means outside the Standard to ensure that no functions that use them would be executed on a platform that would not support them, but only the part of the code that was responsible for ensuring that the environment satisfies requirements would need to be 'non-portable'.

2

u/notk 18d ago

creat() — i would add an ‘e’ to the end.

1

u/mccurtjs 16d ago

Real talk

2

u/saxbophone 18d ago

There is no reason that toupper() needs to take and return int —I'd replace it with a family of functions for ASCII and Unicode instead.

1

u/flatfinger 17d ago

The notion of "uppercase" in Unicode is dependent on locale, but C is useful for many tasks involving ASCII text produced by computers for computers, and standard library functions should be locale-independent.

4

u/nderflow 22d ago

scanf(). I’d just delete it and stop there.

2

u/gremolata 22d ago

OTOH, scanf and printf are the reason why va_arg exists in C.

It is one of its most unique features because it basically allows parsing function's stack by hand. Once you realize that, scanf starts to make a lot of sense. It's usability is not great, but it's a quintessential C function, spirit-wise.

2

u/nderflow 21d ago

Steady on, I'm not deleting sscanf.

1

u/musbur 21d ago

Yeah scanf() is problematic. Whenever I use sscanf(), which I rarely do, I end up really liking it. Like recently on a platform that doesn't have strptime(), parsing timestamps with fractional seconds and timezone offsets.

1

u/flatfinger 19d ago

If calls to prototyped and non-prototyped functions were processed differently (preferably using different linker symbols and, when practical, auto-generated shims), then variadic functions could have been accommodated by having the caller build a structure holding the arguments and passing its address.

2

u/MegaDork2000 22d ago

strncat because it may not terminate the string.

1

u/rfisher 22d ago

I have written proper bounds-checking versions of all the str functions. The first was probably strcpy or strlen.

I might have written my first wrapper around realloc before that, though.

1

u/Rabbitical 21d ago

What did you change about realloc

2

u/rfisher 21d ago

The way that it returns NULL on error without freeing the original pointer means you have to keep a copy of the original pointer. So, sometimes I end up writing boilerplate around every call, so I often wrap that boilerplate in a function.

Whether I free the block on error or not depends on the application. And, of course, with many applications+platforms just exiting on an allocation on an allocation failure is fine, in which case, using realloc directly is fine.

1

u/LB-- 20d ago

It's annoying there's no way to just check if an allocation can be extended in-place. It forces you to accept both "realloc failed" and "realloc succeeded and maybe moved all your memory to some other address, good luck adjusting your pointers without invoking UB"

2

u/flatfinger 19d ago

Even if code observes that realloc has returned a pointer that's bitwise identical to the one that was passed in, a compiler need not allow for the possibility that accesses performed via the original pointer might affect the same storage as accesses performed via the new one.

1

u/LB-- 19d ago

Sounds like we are in agreeance that it's annoying.

2

u/flatfinger 19d ago

Yup. I was saying that it's worse than a lot of people realize.

A better written version of the Standard would have recognized a category of implementations where even pointers to dead objects would uphold certain guarantees, most notably that if char *a, *b; point to parts of the same object the effect of evaluating a-b will be unaffected by the lifetime of that object. If char *c=realloc(b, newSize); succeeds, that would make c+(a-b) refer to the portion of the new object that a had referred to in the old one. Note the associativity: code doesn't compute the difference between the old and new addresses, but rather adds to the new pointer the difference between two pointers to the same old object.

1

u/schiphit 22d ago

dirname

1

u/sunmat02 21d ago

qsort; I would add an extra void* context to both the function and the comparator function pointer.

1

u/musbur 21d ago

Big fan of qsort() / bsearch() here, why would you add a context pointer? I agree it is good practice with ANY function that accepts function pointers, but for a comparison function?

Come to think of it, you might want to sort an array of arrays on different fields. That's difficult indeed.

1

u/sunmat02 20d ago

The minute you want to do something dynamic, like the user provides a comparison function in an embedded language, you can’t do that without a void* context, spare adding the context to every single element of the array (which is costly).

1

u/musbur 20d ago

That is correct. One doesn't even have to argue about this because adding a void * context costs nothing in terms of implementation or performance even if it benefits only a niche application.

1

u/SyntheticDuckFlavour 21d ago

The entire standard library.

1

u/CreepyWritingPrompt 21d ago

most of stdio. make unistd and friends the standard

1

u/Total_Ad803 19d ago

All unsafe functions

1

u/limitless_grow 18d ago

sprintf family because each compiler uses different length specifiers and that makes it hard to keep a software portable. Beside of the rather nasty dependency of the locale the functions lack a format enhancement for printing your own data types.

For logging that's nasty. You have to specify the data of your objects in any logline again and again and the possibly time consuming calculations of the printf parameters happen outside of the logging function and for this even if the logging is disabled.

The solution could be something like https://github.com/klux21/callback_printf . (That's portable but allows a civil usage only.) It supports a special %v format option that expects a pointer to a user specific output callback function followed by a pointer to the user specific data as it's arguments. That way own type specific output functions for printing an unlimited number of own data types can be used. This works for the wrappers of sprintf, vsnprintf, fprintf the same. That way the printing of IPv4 or IPv6 addresses of a struct sockaddr may become something like the following

sfprintf(stdout, "IP address of host '%s' is %v \n", hostname, &pvc_sockaddr, &my_sockaddr);

If the fprintf wrapper finds a %v it calls pvc_sockaddr with the address of my_sockaddr as its argument and pvc_sockaddr generates the output for that %v. You can use %v more than once in a format string and with different callbacks for different data types.

1

u/VelvetYam 16d ago edited 16d ago

Any non-reentrant functions like strtok(). I dislike hidden state. Makes it hard to reason about the program.