stupid ptr problem

[ppetree]

I should know this... sigh...

First, the definition:

typedef struct _task {
 unsigned effective_date;
 unsigned expiration_date;
 FILETEST filetest;
 BOOL bPrevjob;
 char warning_time;
 char snooze_time;
} TASK, *PTASK;

Now, the variables:

static FILETEST fTest;  // local dialog working copy
static PTASK pTask;     // a ptr to the master task

Then, the code:

pTask = (PTASK)(LPARAM)lParam;
memcpy(&fTest, pTask->filetest, sizeof(FILETEST));

Compiler generates an error telling me parameter 2 is not a pointer which is obviously is as pTask is defined as a PTASK which is defined as *PTASK

What am I missing here?

Thanks,

Phil

 

[Salem]

It's not complaining about pTask being a pointer, it's pTask->filetest which isn't a pointer.

Maybe [tt][red]&[/red]pTask->filetest[/tt]


--
If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.

 

[ppetree]

Yeah, that's what I thougth too and tried the & and it didn't give me a valid address... (didn't gpf either, just didn't give me what I know to be contained fTest.lpstrFilename.

hmmm...

 

[xwb]

What is the definition of filetest. Are you doing a shallow copy?

 

[ppetree]

FILETEST is defined as:

typedef struct _filetest {
 int   nAction;    // exists, notexists, size<>, date<>
 int   nFormula;   // a date formula
 unsigned long lValue;  // size/date to check for
 unsigned long lResult; // the results of out test
 BOOL bStatus;          // TRUE if ok, FALSE if test failed
 LPSTR lpstrFileName;   // ptr to the file name
} FILETEST, *PFILETEST;

fTest is a local working copy of pTask->filetest so I copy all the existing contents of pTask->filetest to fTest make changes according to the user edits in the dialog and if the user clicks OK I'll copy fTest back to pTask->filetest. If the user clicks Cancel then pTask->filetest stays intact and the contents of fTest are lost.

 

[Salem]

> LPSTR lpstrFileName;
Except this isn't a local copy of the string being pointed to, it's just a clone of the pointer.

So modifying your local copy also modifies the original.

<soapbox>
Of course, if this were really written in C++, there would be no icky char pointers (it would all be std::string), and each struct/class would have a proper copy constructor to make sure that an actual copy was made, and not just a bitwise "shallow copy" clone.
</soapbox>



--
If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.

 

[ppetree]

<soapbox>
Not every project one receives is in the language we prefer NOR do we always receive the code in the condition we prefer it to be in.

So, those of us who are stuck, really appreciate the help of those who can help. While wishing those whose only function in life is to carry around their soapboxes would find someone else to bug.
</soapbox>

 

[Salem]

Just because you inherited crap doesn't mean you have to keep wading through it. There are a few very basic things you can do to help fix the problems.

Hell, even if you just use the defaults to emit diagnostics when you do something dumb, it would be a start.

#include <iostream>
#include <cstring>
using namespace std;

// struct foo { private: };  is the same as class
// class foo { public: };    is the same as struct
// They're the same thing, except for the default access rights
// so adding all the wonderful things like constructors
// is NOT an excuse for improving the C++.

struct foo {
#if defined(IMPROVE)
    // now add basic class operators
    foo ( ) {                   // default ctor
        a = 0;
        b = NULL;
    }   
    ~foo ( ) {                  // default dtor
        delete [] b;
    }      
    foo (const foo &p) {        // copy ctor - a "deep" copy
        a = p.a;
        b = new char[ strlen(p.b) + 1 ];
        strcpy( b, p.b );
    }
    foo& foo::operator= (const foo& p) {    // assignment
        if ( this != &p ) {                 // not doing var=var;
            a = p.a;
            delete [] b;
            b = new char[ strlen(p.b) + 1 ];
            strcpy( b, p.b );
        }
        return *this;
    }
#endif

    // Now start migrating the data to being private
    // with proper accessor functions to get at the data
    // For example, start with the ones which are pointers, so
    // you can start to validate them, and make sure they're
    // pointing to enough space.  Once private, you can then
    // easily turn a char* into a std::string for even more
    // safety.
#if defined(STEP2)
    void setString ( const char *msg ) {
        delete [] b;
        b = new char[ strlen(msg) + 1 ];
        strcpy( b, msg );
    }
    const char *getString ( void ) {
        return b;
    }
#endif
    int     a;

#if defined(STEP2)
  private:
#endif
    char    *b;
};

foo func1 ( void ) {
    foo var;
    var.a = 1;
#if !defined(STEP2)
    // we don't need this anymore, all the access methods take
    // care of it for us.
    var.b = new char[10];
#endif
    return var;
}

int main ( ) {
#if !defined(STEP2)
    foo v1 = func1();
    strcpy( v1.b, "hello" );
    foo v2 = v1;                // is this a deep or shallow copy?
    strcpy( v2.b, "world" );
    cout << v1.b << endl;       // Is this what you expect?
#else
    foo v1 = func1();
    v1.setString( "hello" );    // Now use accessors
    foo v2 = v1;
    v2.setString( "world" );    // Now use accessors
    cout << v1.getString() << endl;
#endif
    return 0;
}

[tt]
$ g++ foo.cpp
$ ./a.exe
world
[/tt]
This is basically the problem you have with filetest and lpstrFileName. You're copying the pointer, but not what it's pointing at. So modifying one trashes the other.

[tt]
$ g++ -DIMPROVE foo.cpp
$ ./a.exe
hello
[/tt]
Well it's a start. At least it now prints out the expected result.

[tt]
$ g++ -DIMPROVE -DSTEP2 foo.cpp
$ ./a.exe
hello
[/tt]
We're on our way to making all the data private to the class. Start with the pointers, as they're the ones most sensitive to mistakes in shallow copying, not being allocated or not being freed.

Then it's a process of stepwise refinement
- make a member variable private
- compile it and see what complains
- write suitable access methods
- retest the code.

See - you don't have to blindly accept a 'struct' and pretend there is nothing you can do to help yourself overcome your code problems.


--
If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.