1

I have the following struct in my code:

typedef struct
{
    uint64_t pulse;
    fp_t volume;
    fp_t factor; // Factor en pulsos / unidad de volumen
    char *unit_volume;
    char *unit_time;
} totalizer_t;

and it is initialized as static with 3 different variables:

static totalizer_t acm_config =
{
    .pulse = 1000,
    .volume.num = 1,
    .volume.res = 1,
    .factor.num = 1000, // pulsos/unidad_volumen.
    .factor.res = 0,
    .unit_volume = "LT",
    .unit_time = "S",
};

static totalizer_t rate_config =
{
    .pulse = 0,
    .volume.num = 0,
    .volume.res = 1,
    .factor.num = 1000,
    .factor.res = 0,
    .unit_volume = "LT",
    .unit_time = "S",
};

static totalizer_t ttl_config =
{
    .pulse = 12000,
    .volume.num = 1,
    .volume.res = 1,
    .factor.num = 1000, // pulsos/unidad_volumen.
    .factor.res = 0,
    .unit_volume = "LT",
    .unit_time = "S",
};

I want to modify the parameters .unit_volume and .unit_time from each struct. Because of this, I've made 2 functions like this:

void fm_factory_modify_units_time(char *p_str_time_units)
{
    strncpy(acm_config.unit_time, p_str_time_units, 1);
    strncpy(rate_config.unit_time, p_str_time_units, 1);
    strncpy(ttl_config.unit_time, p_str_time_units, 1);
}

void fm_factory_modify_units_volume(char *p_str_volume_units)
{
    strncpy(acm_config.unit_volume, p_str_volume_units, 2);
    strncpy(rate_config.unit_volume, p_str_volume_units, 2);
    strncpy(ttl_config.unit_volume, p_str_volume_units, 2);
}

unit_time is always a single char string, and unit_volume is always a 2 char string.

But when I try to use this function like this:

fm_factory_modify_units_volume("M3");

the acm_config.unit_volume doesn't change it's initial value, it remains being 'LT'. The same happens with the other function for the time units. I can change the other values from the struct. For example if I put rate_config.volume.res = 3 it changes from 1 to 3. So the problem is with the strings.

Using a debugger I can see when it enters the function and I know that the parameter p_str_volume_units is well assigned as for example, the string "M3".

I have tried to use other string.h functions to copy one string into the other like strcpy without the length check, or snprintf/sprintf like the following:

sprintf(rate_config.unit_volume, "%s", p_str_volume_units)

but it's still the same.

I want to use strings because it is compatible with other libraries I have made, but if it is impossible to change them, I will have to change it to an enum or something like that, but it will be hard.

If someone knows how to solve this problem, I will be very grateful.

santiago deliotte
  • 11
  • 1
  • `strncpy` doesn't copy strings. Don't use it. Use `strlcpy` if you can. – Steve Summit Jun 01 '23 at 16:01
  • 6
    Surprised you don't get a segfault from trying to overwrite read only memory, but that's undefined behavior for you – Shawn Jun 01 '23 at 16:04
  • @SteveSummit It's still not working using strlcpy – santiago deliotte Jun 01 '23 at 16:05
  • @santiagodeliotte Shawn's comment is right. – Steve Summit Jun 01 '23 at 16:05
  • For starters, change the definition of those fields in the struct to appropriately sized `char` arrays. – Shawn Jun 01 '23 at 16:07
  • If you're declaring strings as arrays, you can't use assignment; you must generally use `str*cpy`. But if you're declaring strings as pointers, you *can* use assignment (and you generally don't want to use `str*cpy`). Please read [this question](https://stackoverflow.com/questions/48734000) and its answers. – Steve Summit Jun 01 '23 at 16:07
  • Your unit_volume and unit_time are pointers, not char arrays. They can point to char arrays that can contain chars - they cannot and do not contain chars themselves. You initialize them to point to string literals. String literals are constant and are not to be modified. Trying to strncpy to the address of a string literal is a bug, undefined behavior, and something the compiler will usually not detect. – Avi Berger Jun 01 '23 at 16:12
  • 1
    Also, string literal can be reused by the implementation. So your initial 3 instances of unit_volume could all point to the same memory - or not. Your tool chain decides that, not you (it is a memory space optimization). @Shawn's suggestion of using char arrays instead will give you independent and modifiable memory for each instance. What you are expecting and probably want here. – Avi Berger Jun 01 '23 at 16:28
  • Given `char *unit_volume`, the `unit_volume` field is a *pointer*. It can not hold a string by itself - it has to *point* to a string *somewhere else* in memory. Where do you think it's pointing? – Andrew Henle Jun 01 '23 at 16:46
  • Ok I think I understand the lesson, I will try to use well sized arrays when I want to complete them to avoid this kind of behavior. Thank you all. – santiago deliotte Jun 01 '23 at 16:58
  • There may well be better duplicates than the one I just used - feel free to ping me if you find such, and I'll modify the target list. – Adrian Mole Jun 01 '23 at 16:58

0 Answers0