3
\$\begingroup\$

In want to implement length prefixed strings in C (not null terminated), with some idiosyncrasies: malloc is prohibited, memory efficiency is important, each string (except for literal-backed ones) is modifiable and can change its length, but it has a fixed (at declaration) allocated length (maximum 255), C code is generated by a code generator. I want this to work on as many architectures and C compilers as possible.

Suppose I do the following:

typedef struct PLS { 
   uint8_t maxlen; 
   uint8_t curlen; 
   char buf[1];  // actual allocated size is in maxlen 
} PLS ; 

Then I could declare, say

  struct {
    ...
    PLS s1;   // real allocated size: 100
    char s1buf[99];
    PLS s2;  // real allocated size: 64 
    char s2buf[63];
    ...
    };

   // initialization:
   mystruct.s1.maxlen=100;
   mystruct.s1.curlen=0;
   mystruct.s2.maxlen=64;
   mystruct.s2.curlen=0;

   // PLS library:

   void PLS_copy(PLS *to,PLS *from) {
       int tc = from-> curlen; // chars to copy
       if(tc > to->maxlen) tc = to->maxlen;
       memcpy(to->buf,from->buf,tc);
   }

Does this look ok? Any pitfalls? Would some compiler give warnings? I believe that data structure alignment should not padding between s1 and s1buf, but even then it should work, right?

For literals, if I want to avoid a duplication of allocated chars, things seem trickier (remember, though, that code is machine-generated). Would this be objectionable?

 // static declaration of literal "hello" (5 chars) trailing null byte wasted
 char lit1[] = "\x05\x05hello"; 
// can the above be casted to PLS? (I won't modif it, I swear)
 PLS * lit1Pls = &(PLS)(lit1);  // smells funny, though... 
\$\endgroup\$

closed as off-topic by vnp, SuperBiasedMan, Hosch250, ferada, Quill Sep 15 '15 at 22:04

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ Accessing beyond an array of length 1 causes undefined behaviour. You seem unaware , but the C standard is very strict about array bounds. The length-1-and-read-off-the-end hack that was "popular" in the 1980s was never legal and modern compilers will break the code. Instead you could use the flexible array member. See here or browse that tag for other examples. \$\endgroup\$ – M.M Sep 15 '15 at 15:52
  • \$\begingroup\$ Minor idea: void PLS_copy() could return int to indicate if the copy was completely successful. \$\endgroup\$ – chux Sep 15 '15 at 21:23
2
\$\begingroup\$

Copy function improvements

Your copy function could be better:

   void PLS_copy(PLS *to,PLS *from) {
       int tc = from-> curlen; // chars to copy
       if(tc > to->maxlen) tc = to->maxlen;
       memcpy(to->buf,from->buf,tc);
   }
  1. You could use const on your from argument.
  2. You never update the length of the to string. I'm assuming this is like a strcpy, so the to string should have its length set to the length that you just copied.

Here is how I would write it:

   void PLS_copy(PLS *to, const PLS *from)
   {
       uint8_t bytesToCopy = from->curlen;

       if (bytesToCopy > to->maxlen)
           bytesToCopy = to->maxlen;
       memcpy(to->buf, from->buf, bytesToCopy);
       to->curlen = bytesToCopy;
   }

Flexible array member

I agree with the comment by M.M about using flexible array members instead of buf[1]:

typedef struct PLS { 
   uint8_t maxlen; 
   uint8_t curlen; 
   char buf[];  // actual allocated size is in maxlen 
} PLS;

struct {
    ...
    PLS s1;   // real allocated size: 100
    char s1buf[100];
    PLS s2;  // real allocated size: 64 
    char s2buf[64];
    ...
};

Also, you could use a macro to help declare these fields. Something like this:

#define PLS_FIELD(name, length)  \
    PLS name;                    \
    char name##buf[length]

Then you could define your struct as:

struct {
    ...
    PLS_FIELD(s1, 100);
    PLS_FIELD(s2, 64);
    ...
};

Casting literal

For your literal casting, I would do this:

char lit1[] = "\x05\x05hello";
const PLS *lit1Pls = (const PLS *) lit1;

The const keyword here will prevent you from writing to the literal. And the cast I used is simpler than the one you used. I don't see anything objectionable about doing this.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.