Why is a struct member uninitialized after dereferencing the struct

Question

I'm writing an HBridge class to make handling HBridges one arduino much easier. I'm not sure why I call .print() on my HBridge instance, it prints:

---------
BRIDGE ONE
Bridge Use:->pin: 1392293344
Bridge Idle->pin: 1392293408
Freq:0
Dir:0
---------
BRIDGE TWO
Bridge Use:->pin: 8
Bridge Idle->pin: 214125355
Freq:0
Dir:0

Notice how the Bridge Use and Bridge Idle is uninitialized. Am I making a mistake somewhere with my pointers?

Here is my code.

HBridge.h

//-----------------------|
#define true 1         //|
#define false 0        //|
//-----------------------|
#define PIN_OFF 0      //|
#define PIN_ON  1      //|
//-----------------------|
#define PIN_FORWARD 0 //|
#define PIN_BACKWARD 1//|
//-----------------------|



typedef struct{
    int pin;
} Pin;

typedef struct {
    Pin *inuse;
    Pin *idle;
    int freq;
    int direction;
}Bridge;

typedef enum{ A, B} hbridge_pins;

class HBridge {
    private:
    Bridge bridge_a, bridge_b;
    Bridge *pins[2];
    public:
        HBridge(int, int, int, int);
        void setPinFrequency(hbridge_pins, int);
        void setPinDir(hbridge_pins, int);
        void turnPinOFF(hbridge_pins);
        void update(void);
        void print();
};

HBridge.cpp

#import "./HBridge.h"
#import <stdio.h>


Pin pinWith(int num) {
    // pinMode(num, OUTPUT);
    Pin p;
    p.pin = num;
    return p;
}

void swap_pins(Pin *a, Pin *b) {
        Pin temp = *a;
        *a = *b;
        *b = temp;
}

void print_pin(const char * name, Pin *p){
    printf("%s->pin: %d\n",name,p->pin);
}
void print_bridge(Bridge *b){
    print_pin("Bridge Use:", b->inuse);
    print_pin("Bridge Idle", b->idle);
    printf("Freq:%d\nDir:%d\n",b->freq,b->direction);

}

void HBridge::turnPinOFF(hbridge_pins pin) {
    pins[pin]->freq = PIN_OFF;
}

HBridge::HBridge(int pinAA, int pinAB, int pinBA, int pinBB) {

    Pin a_use = pinWith(pinAA);
    Pin a_idle = pinWith(pinAB); 
    Pin b_use = pinWith(pinBA);
    Pin b_idle = pinWith(pinBB);


    bridge_a.inuse = &a_use;
    bridge_a.idle = &a_idle;

    bridge_b.inuse = &b_use;
    bridge_b.idle = &b_idle;
    



    /*---(DEFAULT DIRECTIONS)---*/
    bridge_a.direction = PIN_FORWARD;
    bridge_b.direction = PIN_FORWARD;
    bridge_a.freq = PIN_OFF;
    bridge_b.freq = PIN_OFF;
    
    /*---(ARRAY OF POINTERS TO THE TWO PINS FOR EASY ACCESS BY INDEX)---*/
    pins[0] =&bridge_a;
    pins[1] =&bridge_b;
}

void HBridge::setPinFrequency(hbridge_pins pin, int freq) {
    pins[pin]->freq = freq;
}

void HBridge::setPinDir(hbridge_pins pin, int dir) {
    if ((pins[pin]->direction == PIN_FORWARD && dir == PIN_FORWARD) || (pins[pin]->direction == PIN_BACKWARD && dir == PIN_BACKWARD)) {
        
    } else if (pins[pin]->direction == PIN_FORWARD && dir == PIN_BACKWARD) {
        
        /*----(SWAP POINTERS)----*/
        swap_pins(pins[pin]->inuse, pins[pin]->idle);
        pins[pin]->direction = PIN_BACKWARD;
        
    } else if (pins[pin]->direction == PIN_BACKWARD && dir == PIN_FORWARD) {
        
        /*----(SWAP POINTERS)----*/
        swap_pins(pins[pin]->inuse, pins[pin]->idle);
        pins[pin]->direction = PIN_BACKWARD;
        
    }
}

void HBridge::update(void)/*pointer to an int to save memory*/ {
    /*THE FIRST BRIDGE*/
    // analogWrite(pins[0]->inuse->pin, pins[0]->freq);
    // analogWrite(pins[0]->indle->pin, pins[0]->PIN_OFF);

    // THE SECOND BRIDGE
    // analogWrite(pins[1]->inuse->pin, pins[1]->freq);
    // analogWrite(pins[1]->indle->pin, pins[1]->PIN_OFF);
} 

void HBridge::print(void){

    printf("---------\nBRIDGE ONE\n");
    print_bridge(pins[0]);

    printf("---------\nBRIDGE TWO\n");
    print_bridge(pins[1]);
}


int main(int argc, const char*argv[]){
    HBridge b(31,42,33,4);
    b.setPinFrequency(A,200);
    b.print();
}

`Bridge` should have a constructor that initializes the pointers to `nullptr`. — crashmstr, Mar 17 '15 at 18:59
If you're going to write C++, you need to unlearn all your old C idioms, like `typedef struct { } name;` Just do `struct name { };` Stop using malloc(). Prefer make_unique<>() or make_shared<>() if you actually need dynamic memory. — Rob K, Mar 17 '15 at 21:16
@RobK Thanks for alerting me to this. I'm only about 6 hours into my C++, and am not yet well acquainted will all the features of its standard library. Lesson: before implementing, something, check the docs to see what conventional language X programmers do. — theideasmith, Mar 18 '15 at 01:32
Please don't edit your answer into the question. Instead, post an answer . I have rolled back the question so you can do this (copy-paste out of the edit history if you need to). — M.M, Mar 18 '15 at 02:07

score 2 · Answer 1 · edited May 23 '17 at 12:06

2

Pin a_use is local to the constructor and you are making a pointer from it, although it will be deallocated when the contsructor returns.

Declare inuse and idle as Pin inuse; and Pin idle; and don't make a pointer, just assign to them what pinWith() returns, the value will be copied and the problem will be gone.

If you need them to be pointers, then first request heap space like this

bridge_a.inuse = new Pin;

and then do this

memcpy(bridge_a.inuse, &a_use, sizeof(Pin));

if the Pin doesn't contain any class but just POD fields, it will work, otherwise a more elaborated copy will be needed.

edited May 23 '17 at 12:06

Community

1
1

answered Mar 17 '15 at 18:57

Iharob Al Asimi

52,653
6
59
97

@theideasmith Inside `HBridge`, change `Bridge *pins[2];` to `Bridge pins[2];` (remove the pointer), then in the constructor remove the ampersands in the assignment to `a_use`, `a_idle`, `b_use`, `b_idle`. If you have to take a pointer in `print`, pass the address (`print(&pin[0]); print(&pin[1])` but you really should instead take a reference to a `Bridge` and use `.` instead of `->`. – David G Mar 17 '15 at 19:24

DWright · Answer 2 · 2015-03-17T19:05:18.383

0

Echoing @iharob. You have the same problem in the pinWith() function where you return a Pin off the stack for the pinWith() function, a position which becomes "garbage" once you return from the function. You'll need to allocate a Pin from the heap in pinWith() to fix this.

Pin pinWith(int num) {
    // pinMode(num, OUTPUT);
    Pin p = new Pin;
    p.pin = num;
    return p;
}

edited Mar 17 '15 at 19:05

answered Mar 17 '15 at 18:59

DWright

9,258
4
36
53

Why is a struct member uninitialized after dereferencing the struct

HBridge.h

HBridge.cpp

2 Answers2