0

Hello I got this warning with gcc(version 5.4.0) on a C11 program compiled with the following command:

$ gcc -g -Wall -std=c11 main.c -o minishell   
main.c: In function ‘process_new’:
main.c:184:10: error: assignment of read-only variable ‘s’
        s = slice_next(s, ':')) {

But nothing with clang (version 3.8.0):

$ clang -g -Wall -std=c11 main.c -o minishell # Compile without warning.

I am on Ubuntu 16.04.

Here is the code

// The loop that generate the warning with gcc.
for (str_slice s = slice_at(paths, ':');
       !slice_empty(s);
       s = slice_next(s, ':')) {
//       ^ Gcc complains here.
    const char *full_path = build_full_path(progname, s);
    /* I use with full_path but nothing with s after this point. */ 
    // There is no aliasing on full_path at this point.
    free((void *)full_path); .
  }

And here the definition of str_slice:

typedef struct _str_slice {
  const char* data;
  const uint32_t len; // end - data len of slice.
//^^^^^ Source of gcc warning.
} str_slice;

And the functions to use it:

inline
uint32_t slice_len(const str_slice slice) {
  return slice.len;
}

inline
const char* slice_data(const str_slice s) {
  return s.data;
}

inline
str_slice slice_new(const char* data, uint32_t len) {
  return (str_slice) { data, len };
}

inline
str_slice slice_at(const char* data, const char c) {
  const char* end = strchr(data, c);
  return slice_new(data, end - data);
}

inline
str_slice slice_next(const str_slice s, const char c) {
  const char* data = slice_data(s) + slice_len(s) + 1; // skip c
  const char* end = strchr(data, c);
  if (end != NULL) {
    return slice_new(data, end - data);
  } else {
    return slice_new(NULL, 0);
  }
}

inline
bool slice_empty(const str_slice s) {
  return s.len == 0;
}

And if necessary the code about build_full_path

const char* build_full_path(const char* progname, const str_slice slice) {
  size_t len_progname = strlen(progname);
  // Save additional 2 bytes for adding '/' and '\0'.
  size_t full_path_size = len_progname + slice.len + 2; 
  size_t malloc_size = sizeof(char) * full_path_size;
  char *full_path = malloc(malloc_size);

  full_path[full_path_size - 1] = '\0';
  memcpy(full_path, slice.data, slice.len);
  full_path[slice.len] = '/';
  memcpy(full_path + slice.len + 1, progname, len_progname);

  return (const char *) full_path;
}

When compile with clang I got an executable with the good behavior. So I made something wrong? Or I found a bug?

Here the full code of my program(outdated): https://gist.github.com/darnuria/12af88c509310c2b40e0031522882720

Edit: Use of memcpy instead of strncpy. Remove of const on scalar types.

Darnuria
  • 166
  • 1
  • 10
  • What is the definition of `str_slice`? – Andy Schweig Feb 19 '17 at 00:24
  • @AndySchweig I moved the definition in a separated code block for clarity. :) – Darnuria Feb 19 '17 at 01:27
  • Another blatant misuse of `strncpy`. Use `memcpy` or `snprintf` instead, – chqrlie Feb 19 '17 at 01:34
  • After reading (carefully) the man, I understand why: `strncpy` will put '\0' in my `char*` if len(src) is smaller than slice.len. And because I put myself the null terminating bit I can directly use memcpy. From the man of strncpy: "If the length of src is less than n, strncpy() writes additional null bytes to dest to ensure that a total of n bytes are written." – Darnuria Feb 19 '17 at 01:46

1 Answers1

1

In the structure the data member len is declared as a constant data member.

typedef struct _str_slice {
  const char* data;
  const uint32_t len; // end - data len of slice.
  ^^^^^^     
} str_slice;

It means that it can be changed and as result you may not assign one object of the structure to another object of the structure.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • Ok thanks! But if I want to keep the constness of this field. How can I do? I want to keep the possibility to pass by copy this struct since it's just one pointer and a 32bit unsigned int. – Darnuria Feb 19 '17 at 00:38
  • @Darnuria You can pass by copy because in this case a new temporary object is created. But you may not assign one object of the structure to another object of the structure. If you are going to use the assignment operator then the data member len shall not be const. – Vlad from Moscow Feb 19 '17 at 00:41
  • Ok many thanks! I understand. Should I make a small reproducible example and fill a bug to clang? Seems strange to get no warning in clang. – Darnuria Feb 19 '17 at 00:44
  • @Darnuria I think you may provided that in the used compiler all warnings are switched on. – Vlad from Moscow Feb 19 '17 at 00:46
  • Seems clear after reading https://softwareengineering.stackexchange.com/questions/222652/whats-the-reason-for-c-standard-to-consider-const-ness-recursively – Darnuria Feb 19 '17 at 01:18
  • @Darnuria: There are too many `const` in your code. const qualifying function arguments, local variables and structure members is overkill. – chqrlie Feb 19 '17 at 01:37
  • The clang bug has previously been reported (it crops up on SO regularly) – M.M Feb 19 '17 at 02:30