why does this work?

[Matt Kowalczyk]

Hello,

Can someone explain to me why the following code works correctly? It would seem
like it shouldn't.

irc_client.c contains:

#include "ircclient.h

IRCClient* init_irc_client() {
IRCClient* irc_client;

irc_client = malloc(sizeof(IRCClient));
irc_client->irc_connect_to = irc_connect_to;
// return irc_client;
}

irc_client.h contains:

typedef struct {
/* methods */
int (*irc_connect_to) (xdcc_packet* packet);
} IRCClient;

typedef struct {
char* server;
int port;
char* channel;
char* nick;
int packet_nr;
} xdcc_packet;

My main.c contains:

#include <assert.h>
#include "ircclient.h"

int main(int argc, char* argv[]) {

IRCClient* client;
xdcc_packet packet;

/* packet init code goes here - deleted for clarity */

client = init_irc_client();
assert(client);

client->irc_connect_to(&packet);

free_irc_client(client);
free_xdcc_packet(&packet);
return 0;
}


My question is, init_irc_client() initializes client in the main function
regardless of whether init_irc_client() contains the return statement or not. In
my example code above, I have the return statement commented out, however when I
call client->irc_connect_to, the appropriate function is called. Why is client
getting initialized regardless of the fact that init_irc_client returns a
pointer to a client or not.

The assert(client) statement _only_ fails if I don't make the assignment:
client = init_irc_client();

Thanks,
Matt

 

[mkalyan79]

From my understanding ,
this might not work always, but in cases why it work

irc_client = malloc(sizeof(IRCClient));

init_irc_client() allocated the value of IRCClient* irc_client on heap
and the value lives across function calls so the client after getting
the value initalized might be able to make the call.

But yeah in all cases it should not work. I could be wrong though too
just in case.

-K

 

[Matt Kowalczyk]

this might not work always, but in cases why it work

irc_client = malloc(sizeof(IRCClient));

init_irc_client() allocated the value of IRCClient* irc_client on heap
and the value lives across function calls so the client after getting
the value initalized might be able to make the call.

But yeah in all cases it should not work. I could be wrong though too
just in case.

-K

How could I restructure my code to dynamically create new instances of IRCClient?

So the function IRCClient* init_irc_client() does in fact return a new instance
of a pointer to an IRCClient? I would like the ability to create many instances
of these clients. e.g. multiple calls to init_irc_client() return multiple
instances.

Thanks,
Matt

 

[Jack Klein]

Hello,

Can someone explain to me why the following code works correctly? It would seem
like it shouldn't.

No, it doesn't seem like anything, see below.

irc_client.c contains:

#include "ircclient.h

IRCClient* init_irc_client() {
IRCClient* irc_client;

irc_client = malloc(sizeof(IRCClient));
irc_client->irc_connect_to = irc_connect_to;
// return irc_client;
}

With the return statement commented out, you are hitting the end of
the function without returning a value. When you do this in a C
program for a function defined to return a value, with one exception
that need not concern us here, you are basically "returning" an
indeterminate value.

[snip]

#include <assert.h>
#include "ircclient.h"

int main(int argc, char* argv[]) {

IRCClient* client;
xdcc_packet packet;

/* packet init code goes here - deleted for clarity */

client = init_irc_client();

Here you are using the indeterminate value "returned" by the function,
and this is where your program causes undefined behavior. It is
actually legal in C to have a function not return a value, even though
it says it will, but the moment the caller of the function uses that
value, even just storing it into memory, the behavior is undefined.

assert(client);

This is an invalid use of the assert macro, by the way, unless your
compiler conforms to the C99 standard. The assert macro, prior to
C99, requires an expression of type int, and a pointer is not an int.

client->irc_connect_to(&packet);

free_irc_client(client);
free_xdcc_packet(&packet);
return 0;
}


My question is, init_irc_client() initializes client in the main function
regardless of whether init_irc_client() contains the return statement or not. In
my example code above, I have the return statement commented out, however when I
call client->irc_connect_to, the appropriate function is called. Why is client
getting initialized regardless of the fact that init_irc_client returns a
pointer to a client or not.

Undefined behavior is a specific term in the C language standard that
is rather self-explanatory. The language does not place any
requirements on the result. It could "work", it could fail, it could
make pigs grow wings and fly. So there is no point in asking why it
"works", it is not a language issue. Once you generate undefined
behavior anything that happens is just as right, or just as wrong, as
anything else, as far as the language is concerned.

The assert(client) statement _only_ fails if I don't make the assignment:
client = init_irc_client();

See my comments above about the assert macro, and check your
documentation on it.

 

[Barry Schwarz]

Hello,

Can someone explain to me why the following code works correctly? It would seem
like it shouldn't.

If you included the actual code we might be able to.

irc_client.c contains:

#include "ircclient.h

Missing the closing ".

IRCClient* init_irc_client() {
IRCClient* irc_client;

irc_client = malloc(sizeof(IRCClient));
irc_client->irc_connect_to = irc_connect_to;
// return irc_client;

Does you compiler not give a diagnostic?

}

irc_client.h contains:

typedef struct {
/* methods */
int (*irc_connect_to) (xdcc_packet* packet);

Since xdcc_packet is not defined till later, this also must produce a
diagnostic.

} IRCClient;

typedef struct {
char* server;
int port;
char* channel;
char* nick;
int packet_nr;
} xdcc_packet;

My main.c contains:

#include <assert.h>
#include "ircclient.h"

int main(int argc, char* argv[]) {

IRCClient* client;
xdcc_packet packet;

/* packet init code goes here - deleted for clarity */

client = init_irc_client();

At this point you invoke undefined behavior, trying to use the return
value from a function that forgot to return it. The fact that it
appears to work properly is just really bad luck. (I expect the
reason for the apparent success is that the register the value would
have been returned in just happens to have the "correct" value as a
residual side affect of the last statement executed in the function.)

assert(client);

If you delete the assignment above, then client never gets
initialized; its value is indeterminate. Attempting to evaluate an
indeterminate value is more undefined behavior.

client->irc_connect_to(&packet);

free_irc_client(client);
free_xdcc_packet(&packet);
return 0;
}


My question is, init_irc_client() initializes client in the main function
regardless of whether init_irc_client() contains the return statement or not. In
my example code above, I have the return statement commented out, however when I
call client->irc_connect_to, the appropriate function is called. Why is client
getting initialized regardless of the fact that init_irc_client returns a
pointer to a client or not.

Really bad luck, destined to fail only when you demonstrate your
product to an important customer or you submit it for some form of
formal certification. With undefined behavior, good luck is an
immediate software error with a diagnostic that helps you debug the
mistake.

The assert(client) statement _only_ fails if I don't make the assignment:
client = init_irc_client();

Thanks,
Matt

Remove del for email

 

[Matt Kowalczyk]

Hello,

Can someone explain to me why the following code works correctly? It would seem
like it shouldn't.

No, it doesn't seem like anything, see below.

irc_client.c contains:

#include "ircclient.h

IRCClient* init_irc_client() {
IRCClient* irc_client;

irc_client = malloc(sizeof(IRCClient));
irc_client->irc_connect_to = irc_connect_to;
// return irc_client;
}

With the return statement commented out, you are hitting the end of
the function without returning a value. When you do this in a C
program for a function defined to return a value, with one exception
that need not concern us here, you are basically "returning" an
indeterminate value.

[snip]

#include <assert.h>
#include "ircclient.h"

int main(int argc, char* argv[]) {

IRCClient* client;
xdcc_packet packet;

/* packet init code goes here - deleted for clarity */

client = init_irc_client();

Here you are using the indeterminate value "returned" by the function,
and this is where your program causes undefined behavior. It is
actually legal in C to have a function not return a value, even though
it says it will, but the moment the caller of the function uses that
value, even just storing it into memory, the behavior is undefined.

assert(client);

This is an invalid use of the assert macro, by the way, unless your
compiler conforms to the C99 standard. The assert macro, prior to
C99, requires an expression of type int, and a pointer is not an int.

client->irc_connect_to(&packet);

free_irc_client(client);
free_xdcc_packet(&packet);
return 0;
}


My question is, init_irc_client() initializes client in the main function
regardless of whether init_irc_client() contains the return statement or not. In
my example code above, I have the return statement commented out, however when I
call client->irc_connect_to, the appropriate function is called. Why is client
getting initialized regardless of the fact that init_irc_client returns a
pointer to a client or not.

Undefined behavior is a specific term in the C language standard that
is rather self-explanatory. The language does not place any
requirements on the result. It could "work", it could fail, it could
make pigs grow wings and fly. So there is no point in asking why it
"works", it is not a language issue. Once you generate undefined
behavior anything that happens is just as right, or just as wrong, as
anything else, as far as the language is concerned.

The assert(client) statement _only_ fails if I don't make the assignment:
client = init_irc_client();

See my comments above about the assert macro, and check your
documentation on it.

Thanks, your comments helped me restructure my code to something that is
hopefully defined.

my init_irc_client implementation now looks like this:

IRCClient* init_irc_client();

IRCClient* init_irc_client() {
IRCClient* client;

client = malloc(sizeof(IRCClient));
if (!client)
return NULL;

memset(client, 0, sizeof(IRCClient));

client->irc_connect_to = irc_connect_to;

return client;
}

I then initialize my clients like so:

client = init_irc_client();
client1 = init_irc_client();

My assumption and intentions are that client and client1 are two sepretate
instances of an IRCClient. Modiying one should not effect the other.

Instead of the assert statement, I have changed it to

if (client == NULL)
/* failure */

init_irc_client will return NULL if something has gone wrong. Is there a better
way to handle error reporting?

One way I can think of is to modify the function signature to return an int
which would provide me with a framework to return different types of errors.
Another method I can think of is to use errno. Is it appropriate to use both?


Thanks,
Matt

 

[Flash Gordon]

Matt Kowalczyk wrote:

Thanks, your comments helped me restructure my code to something that is
hopefully defined.

my init_irc_client implementation now looks like this:

IRCClient* init_irc_client();

If a function takes no parameters it is better to say so, then the
compiler is required to issue a diagnostic if you try to pass it something.
IRCClient* init_irc_client(void);

Also, I assume the above is in a header file.

IRCClient* init_irc_client() {
IRCClient* client;

client = malloc(sizeof(IRCClient));

You can combine the declaration and assignment, also there is a more
maintainable form:
IRCClient* client = malloc(sizeof *client);

if (!client)
return NULL;

memset(client, 0, sizeof(IRCClient));

Again, using sizeof *client would be better, however you could have
other problems. The above is not guarantees to set any pointers in
IRCClient to null pointers or floating point numbers to 0. If it really
is the correct thing you could have used calloc to allocate the memory,
if not there are other ways.

client->irc_connect_to = irc_connect_to;

return client;
}

I then initialize my clients like so:

client = init_irc_client();
client1 = init_irc_client();

My assumption and intentions are that client and client1 are two
sepretate instances of an IRCClient. Modiying one should not effect the
other.
Correct.

Instead of the assert statement, I have changed it to

if (client == NULL)
/* failure */

init_irc_client will return NULL if something has gone wrong. Is there
a better way to handle error reporting?

Currently the only thing that can go wrong is a malloc failure, so
returning a null pointer on failure is as good as any.

One way I can think of is to modify the function signature to return an
int which would provide me with a framework to return different types of
errors. Another method I can think of is to use errno. Is it
appropriate to use both?

You really need to look at your entire error handling strategy rather
than just one function and decide what will work best for your project.