How do you optimize this?

Question

I have a struct like this:

static struct F_t {
  char *_1;
  char *_2;
  char *_3;
  char *_4;
  char *_5;
  char *_6;
  char *_7;
  char *_8;
  char *_9;
  char *_10;
  char *_11;
  char *_12;
} F = { 0 };

According to some values from stdin, it's set properly.

The "problem" is that I retrieve it from string, and I need set it properly. I'm using this function currently:

static inline void 
setf(int i, char *value)
{
  /* Nothing to do. */
  if(i > 12)
    return;

  if(i == 1)
    F._1 = value;
  else if(i == 2)
    F._2 = value;
  else if(i == 3)
    F._3 = value;
  else if(i == 4)
    F._4 = value;
  else if(i == 5)
    F._5 = value;
  else if(i == 6)
    F._6 = value;
  else if(i == 7)
    F._7 = value;
  else if(i == 8)
    F._8 = value;
  else if(i == 9)
    F._9 = value;
  else if(i == 10)
    F._10 = value;
  else if(i == 11)
    F._11 = value;
  else if(i == 12)
    F._12 = value;
}

I've tried by using macros.. but as it's set in run-time, it will not possible. I know that if have no reflections, modern things like this, etc. But maybe there is something that I do not know. Probably it's possible in C++. But no. I want pure C.

Any suggestion is very appareciated. Thanks in advance.

The source code might look a little clunky, but I'll bet the compiler does the exact right thing behind the scenes. — ams, Jul 27 '12 at 15:32
@ams - My copy of GCC 4.4 doesn't do it with `-O3`. It generates 12 basic blocks for the assignment, and it generates 12 conditional jumps to these. — ArjunShankar, Jul 27 '12 at 15:36
Disappointing. I thought the compiler would convert this into a switch table and DTRT, but it doesn't. I've tested 4.6.3 and still no joy. — ams, Jul 30 '12 at 08:03

score 5 · Accepted Answer · answered Jul 27 '12 at 15:27

5

Why don't you just do this:

static struct F_t {
    char ** _;
} F = { 0 };

At this point, just make sure you malloc for the double array of size 12 (in your case) and then you can have a function as follows:

setf(int i, char *value)
{
     F._[i] = value;
}

Things look a lot better no?

answered Jul 27 '12 at 15:27

Florin Stingaciu

8,085
2
24
45

3

I like this answer. But why `malloc` when you can declare an array of 12 pointers in the struct with `char * _[12]`? – ArjunShankar Jul 27 '12 at 15:33
I agree that `char * _[12]` will be better. And preferably some name for it too. – Michał Górny Jul 27 '12 at 15:38
@ArjunShankar I agree with both of you guys. I just figured its a little bit more useful that way. – Florin Stingaciu Jul 27 '12 at 15:40
@MichałGórny And as far as the name goes I just tried to be consistent. – Florin Stingaciu Jul 27 '12 at 15:41
1

@Flo - If the OP is already creating a struct with 12 elements, then you can get no additional usefulness by using a char ** instead of a char*[12]. The OP wants, forever, to use 12 elements, no more, no less. Instead, char** leaves you open to memory-management bugs. – ArjunShankar Jul 27 '12 at 15:44

score 3 · Answer 2 · edited Jul 27 '12 at 15:53

3

Besides strongly advising considering @PhillipNordwall's suggestion of using an array. This could work assuming the compiler will packs the structure fields without any sort of padding between them.

static inline void 
setf(int i, char *value)
{
   char **p = &F._1;  /* obtain the address of the first field in the struct */
   p+= i - 1;         /* add an offset to the address according to index */
  /* Nothing to do. */
  if(i > 12)
    return;

   *p = value;        /* set the field's value */
}

edited Jul 27 '12 at 15:53

Phillip Nordwall

465
3
12

answered Jul 27 '12 at 15:29

sorry, should declare the p variable before the if statement, C++ style declarations are hard to give up ;) – Jul 27 '12 at 15:38
@Kakashi, these days I hardly program on C or C++ anymore, I still love those languages though - (C a bit more than C++ - modern C++ with strong template metaprogramming is very hard for me to digest). It's nice to learn that C has adopted C++ style declarations, I like my variables near the code using them... ;) – Jul 27 '12 at 15:48
I'm sorry for the typo in your name Phillip! Thanks for the edits. – Jul 27 '12 at 15:55

kosteg · Answer 3 · 2012-07-27T17:02:15.087

1

Try this:

static inline void  setf(int i, char *value) 
{
    char** F_as_array = (char**)(&F._1);

    if (i > 12 || i < 1) 
        return;

    F_as_array[i-1] = value; 
}

(I have not compiled it, but i think it works)

Edit: F_as_array[i] -> F_as_array[i-1] (From the code you provided, I assume that the first index is 1)

edited Jul 27 '12 at 17:02

answered Jul 27 '12 at 15:29

kosteg

46
5

@MichałGórny - I'm too lazy to read the C99 PDF now to verify if you're right, but look at [this question](http://stackoverflow.com/q/5524552/274261). If you're still sure it will fail, you should consider posting a correct answer to that question as well. – ArjunShankar Jul 27 '12 at 15:51
I assumed something like this a long time ago as well but modern compiler don't seem to strictly adhere to this. It is just a *very bad practice* to cast one object of a completely different type to another even if any standards-compliant compiler should be able to handle it. – Michał Górny Jul 27 '12 at 16:16
I have to agree with MichałGórny that it is a bad practice, for the reason he mentioned (alignment). But, unfortunately, I have worked in some projects that used an automatic code generation tool for declaring global data-variables. (It was a constraint of the project.) The tool doesn't declare arrays at all. That's why the only way for us was to disable the alignment and use the solution I've provided here. In order to do it in GCC compiler: `#define packed_data __attribute__((__packed__))`. Just a small fix, i didn't notice that the first index is '1'. I'll fix it in my original answer. – kosteg Jul 27 '12 at 16:56

How do you optimize this?

3 Answers3