Segmentation fault when using setenv (C)

Question

This is a part of my code (where max is a float):

        printf("noise level found: %f\n", max);

        //Put into "String"
        char var[21];
        sprintf(var, "%f", max);

        setenv("music_sync_soundcard_noise_level",var,1);

        printf("noise level written\n");

Which produces the output:

noise level found: 2410965368832.000000
Segmentation fault

While some lines earlier I have almost the same:

printf("test finished, offset is %f\n", *offset);

//Put into "String"
char var[20];
sprintf(var, "%f", *offset);

setenv("music_sync_soundcard_offset",var,1);

Which works without a problem.

//EDIT: Changed Arraysize, sadly didn't fix problem

Also, I would be careful with using `setenv` with a stack value. — Jazzwave06, Feb 13 '20 at 15:52
by my count, "2410965368832.000000" is 20 characters, so you're 1 short (need room for the NUL terminator). Unless you're on an embedded system with extremely limited resources, memory is cheap and abundant, don't worry about trying to save a few bytes, `char var[64]` or `[128]` should be more than enough and your memory usage won't even blink. — yano, Feb 13 '20 at 15:56
why and how do I set an environment variable to a value computed by my code? — Potheker, Feb 13 '20 at 15:56
@sturcotte06 `setenv` should be OK with stack values as it makes copies of the strings. (It's `putenv` you need to be careful with, because `putenv` stores the actual `char *` value.) — Ian Abbott, Feb 13 '20 at 15:59
@yano "char var[64] or [128]" --> or perhaps `char var[1 + 1 + DBL_MAX_10_EXP + 1 + 6 + 1]` and cope with all `double` with `"%f"`. — chux - Reinstate Monica, Feb 13 '20 at 16:05
@chux-ReinstateMonica Sure, don't know what all your magic numbers are there, but if that's representative of the max string length a `double` can be, that's the best solution. — yano, Feb 13 '20 at 16:08
If nothing else, use `snprintf` so that if the buffer is too small it's truncated rather than smashing the stack... — R.. GitHub STOP HELPING ICE, Feb 13 '20 at 16:08
Likely OP has a crash in the code that follows that they're not showing us... — R.. GitHub STOP HELPING ICE, Feb 13 '20 at 16:11
We can't really see what you're changing, and we don't know what else in your code may be having an effect. Please create a [mcve]. — Nate Eldredge, Feb 13 '20 at 16:17

chux - Reinstate Monica · Answer 1 · 2020-02-13T20:10:21.377

A insufficient size buffer is undefined behavior (UB). That UB may show itself in places far from the original UB.

Improve code by writing code that will not overrun the buffer - everywhere and for all values.

Provide adequate buffer for all max

#include <float.h>

float max;    
// char var[21];
char var[1 + 1+FLT_MAX_10_EXP                             + 1 + 6      + 1];
//       -   d dddddddddd dddddddddd dddddddddd dddddddd    .   fraction \0
sprintf(var, "%f", max);

Use snprint() to avoid buffer overflow @R..

// sprintf(var, "%f", max);
snprintf(var, sizeof var, "%f", max);

Robust code would check the return value:

int len = snprintf(var, sizeof var, "%f", max);
if (len < 0 || (unsigned) len >= sizeof var) Oops();

Use "%e" to cope with and well present large and small floating point values

Use exponential notation to avoid large buffers. Use enough precision.

char var[1 + 1 + 1 + FLT_DECIMAL_DIG-1 + 1 + 1 + 4 + 1];
//       -   d   .   dddddddd            e   -  expo \0
sprintf(var, sizeof var, "%.*e", FLT_DECIMAL_DIG-1, max);

so I found out that the first setenv (which would seem sucessful) actually is the problem, when I comment it out everything works fine. But I can't get it to work without commenting it out. I tried your tips (although I don't entirely understand than, I'm not experienced) and even when I just write "sprintf(var, "test");" or no sprintf at all, it yields some kind of error (either segfault, realloc(): invalid old size, or so and they change everytime I run) — Potheker, Feb 13 '20 at 17:58
oh wow now I understand the dimension of "UB may show itself in places far from the original UB", I've had a soundcard read with a too short buffer size several lines prior and with a buffer that had nothing to do with the variables, which was causing the problem. Quite fascinating. Thanks! — Potheker, Feb 13 '20 at 18:12

score -2 · Answer 2 · answered Feb 14 '20 at 02:46

-2

Never, ever, use functions like sprintf() and strcpy() which do not take an argument that specifies the size of the buffer. Always use snprintf(), strlcpy(), etc. and check the return value for errors. That way, if the buffer is too small you can handle it rather than just assuming that scribbling on random memory locations past the end of your buffer won't cause any problems. As @chux noted what you're doing results in undefined behavior. Also known as a halt and catch fire (HCF) situation.

Also, note that this isn't due to anything setenv() is doing. The bug is that sprintf() is writing past the end of the buffer you gave it. Literally anything you do after that sprintf() call could fail. Including simply returning from the function that did that sprintf() call.

answered Feb 14 '20 at 02:46

Kurtis Rader

6,734
13
20

1

`strcpy` is fine if you have checked the length of the source – M.M Feb 14 '20 at 03:25
@M.M Wrong. `strcpy()` should never be used. In part because people who use it are still likely to have off-by-one bugs because they don't take into account the terminating zero byte when calculating the length of the source. No competent software engineer would ever introduce its use today. – Kurtis Rader Feb 15 '20 at 01:10
1

Competent software engineers know about null terminators so your statement seems self-contradictory . And an off-by-one error is possible with snprintf, strlcpy, etc. – M.M Feb 15 '20 at 02:15
@M.M I certainly hope you're not writing software for a living. An off by one error, in the usual sense of that term, isn't possible with `snprintf()`, `strlcpy()`, etc. Yes, you can cause problems using those functions by lying about the length of the buffer but that wouldn't normally be considered an off by one error. – Kurtis Rader Feb 15 '20 at 03:03
The great thing with strcpy is that you can't lie about the length of the buffer – M.M Feb 15 '20 at 09:18
[here's an example from today](https://stackoverflow.com/a/60237332/1505939). What changes are you going to recommend to this code with no possible way of failing currently? Use a different function that introduces another point of failure? – M.M Feb 15 '20 at 09:20
@M.M Ha ha ha! You got me :-) Just kidding. Obviously, you think that when I said that `strcpy()` should never be used I meant that applied even in situations such as the contrived example you found. Which is a rather silly interpretation. Especially since that example should just use `strdup()`. It's use of `malloc()` followed by `strcpy()` is rather silly. – Kurtis Rader Feb 16 '20 at 03:54
`strdup` is not in ISO C. And there are plenty of other similar situations where the lengths are known to be safe due to the previous code. – M.M Feb 16 '20 at 06:23
@M.M `strdup()` was recently added to the ISO C standard and I've never encountered a system without it. I've been programming on UNIX based systems since 1985. Regardless, using `strcpy()` is silly since the example you linked to had already determined the length of the string. It would be more efficient to use `memcpy()`. You're also being deliberately obtuse since a phrase like "never use strcpy()" is not intended to mean there is no conceivable situation where it isn't appropriate. Only that it should not be used without a damn good reason. Which is going to be very rare. – Kurtis Rader Feb 17 '20 at 02:27
`strdup` is not in any published ISO C standard (it's slated for inclusion in the next one). You wrote "never, ever" in your answer , that's a pretty strong claim . And "damn good reason" is obviously subjective, as is the definition of "competent software engineer". The compiler should optimize the length count if possible, the point of using `strcpy` in this situation is that it's easy to read and there is no possibility of a mistake, compared to using `memcpy` plus a null terminator set where you could make various length and offset errors. – M.M Feb 17 '20 at 04:14
https://stackoverflow.com/questions/60281429/modifying-a-string-char-array – M.M Feb 18 '20 at 20:08

Segmentation fault when using setenv (C)

2 Answers2