Please review

[Stephen Mayes]

I'm afraid I'm not in school, so I have no-one to ask but you nice folks.
I learned most of this stuff from lurking around here, anyway.

Are there any problems with this code?
It appears to work on the (free) compilers I have.

#include <stdio.h>

struct _my_struct;
typedef void(callback_function) (struct _my_struct * arg);
typedef struct _my_struct
{
int data;
callback_function * cb_func;
} my_struct_t;

void any_callback_function (my_struct_t * b)
{
b->data <<= 1;
}
void test (my_struct_t * a)
{
a->cb_func (a);
}

int main (void)
{
my_struct_t ms = {0};

ms.data = 1234;
ms.cb_func = any_callback_function;

test (&ms);

printf ("%d\n", ms.data);
return 0;
}

 

[E. Robert Tisdale]

I'm afraid I'm not in school, so I have no-one to ask but you nice folks.
I learned most of this stuff from lurking around here, anyway.

Are there any problems with this code?
It appears to work on the (free) compilers I have.

> cat main.c

#include <stdio.h>

typedef struct my_struct_t my_struct_t;
typedef void (*callback_function) (my_struct_t* arg);

struct my_struct_t {
int data;
callback_function cb_func;
};

void any_callback_function(my_struct_t* p) {
p->data <<= 1;
}

void test(my_struct_t* p) {
p->cb_func(p);
}

int main (int argc, char* argv[]) {
my_struct_t ms = {0};

ms.data = 1234;
ms.cb_func = any_callback_function;

test(&ms);

printf("%d\n", ms.data);
return 0;
}

> gcc -Wall -std=c99 -pedantic -o main main.c
> ./main

2468

 

[E. Robert Tisdale]

I'm afraid I'm not in school, so I have no-one to ask but you nice folks.
I learned most of this stuff from lurking around here, anyway.

Are there any problems with this code?
It appears to work on the (free) compilers I have.

cat main.c

#include <stdio.h>

typedef struct my_struct_t my_struct_t;
typedef void (*callback_function) (my_struct_t* arg);

struct my_struct_t {
int data;
callback_function cb_func;
};

void any_callback_function(my_struct_t* p) {
p->data <<= 1;
}

void test(my_struct_t* p) {
p->cb_func(p);
}

int main (int argc, char* argv[]) {
my_struct_t ms = {0};

ms.data = 1234;
ms.cb_func = any_callback_function;

test(&ms);

printf("%d\n", ms.data);
return 0;
}

gcc -Wall -std=c99 -pedantic -o main main.c
./main

2468

Here is a better design:

> cat my_struct_t.h

#ifndef GUARD_MY_STRUCT_T_H
#define GUARD_MY_STRUCT_T_H 1
#include <stdio.h>

typedef struct my_struct_t my_struct_t;
typedef my_struct_t* (*callback_function) (my_struct_t* arg);

struct my_struct_t {
int data;
callback_function cb_func;
};

inline static
my_struct_t my_struct_t_create(
int data, callback_function cb_func) {
my_struct_t ms;
ms.data = data;
ms.cb_func = cb_func;
return ms;
}

inline static
int my_struct_t_fprintf(FILE *stream, const my_struct_t* p) {
return fprintf(stream, "%d", p->data);
}

inline static
void my_struct_t_destroy(const my_struct_t* p) { }

#endif//GUARD_MY_STRUCT_T_H

> cat main.c

#include "my_struct_t.h"

inline
my_struct_t* any_callback_function(my_struct_t* p) {
p->data <<= 1;
return p;
}

inline
my_struct_t* test(my_struct_t* p) {
p->cb_func(p);
return p;
}

int main (int argc, char* argv[]) {
my_struct_t ms = my_struct_t_create(
1234, any_callback_function);

test(&ms);

my_struct_t_fprintf(stdout, &ms);
fprintf(stdout, "\n");

my_struct_t_destroy(&ms);

return 0;
}

> gcc -Wall -std=c99 -pedantic -o main main.c
> ./main

2468

Always define a [pseudo] constructor
which returns an initialized object.

Always define a destructor and call it
before the thread exits the scope of the object
even if the destructor doesn't actually do anything.

Functions which modify an object should return
a pointer to that object instead of void
so that the function can be used in an expression.

Use inline [static] functions so that the compiler
can inline and optimize small functions.

 

[sathya]

I'm afraid I'm not in school, so I have no-one to ask but you nice folks.
I learned most of this stuff from lurking around here, anyway.

Are there any problems with this code?
It appears to work on the (free) compilers I have.

#include <stdio.h>

struct _my_struct;
typedef void(callback_function) (struct _my_struct * arg);
typedef struct _my_struct
{
int data;
callback_function * cb_func;
} my_struct_t;

void any_callback_function (my_struct_t * b)
{
b->data <<= 1;
}
void test (my_struct_t * a)
{
a->cb_func (a);
}

int main (void)
{
my_struct_t ms = {0};

ms.data = 1234;
ms.cb_func = any_callback_function;

test (&ms);

printf ("%d\n", ms.data);
return 0;
}

There is nothing wrong in the above and the out put is exactly as expected.
But the experts
might correct you with the function argument passing, test (&ms). Or they
might not.

--
"Combination is the heart of chess"
A.Alekhine
Mail to:
sathyashrayan25 AT yahoo DOT com
(AT = @ and DOT = .)

 

[Jack Klein]

I'm afraid I'm not in school, so I have no-one to ask but you nice folks.
I learned most of this stuff from lurking around here, anyway.

Are there any problems with this code?
It appears to work on the (free) compilers I have.

#include <stdio.h>

struct _my_struct;

Use of identifiers beginning with an underscore followed by a lower
case letter at file scope invades a namespace reserved by the C
standard. Don't do this.

typedef void(callback_function) (struct _my_struct * arg);

Using typedef to create an alias for a function type is a little
silly, since you can't use the typedef to actually define a function.
Such a typedef can only be used to define pointers to functions. So
you might as well include the pointer in the typedef:

typedef void(*callback_function)(struct ... *arg);

typedef struct _my_struct
{
int data;
callback_function * cb_func;
} my_struct_t;

I generally recommend that a structure type be given either a
structure tag or a typedef, but not both. There are actually a few
cases where it is worthwhile to do both, but this is not one of them.
You could replace everything above with this:

struct my_struct_t;

typedef void(*callback_function)(struct my_struct_t *arg);

struct my_struct
{
int data;
callback_function cb_func;
};

Note that if you do need to, or just want to, provide both a structure
tag and a typedef for a structure type, you don't need to resort so
silly and invalid things like sticking in extra underscores. Struct
tags and typedefs live in two different namespaces, so you can just do
this:

typedef struct my_struct_t my_struct_t;

Now with or without the keyword 'struct', 'my_struct_t' refers to the
same type.

void any_callback_function (my_struct_t * b)
{
b->data <<= 1;
}
void test (my_struct_t * a)
{
a->cb_func (a);
}

int main (void)
{
my_struct_t ms = {0};

The line above is a little silly give the next two lines below. It
makes more sense to just code:

my_struct_t ms = { 1234, any_call_back_function };

ms.data = 1234;
ms.cb_func = any_callback_function;

test (&ms);

printf ("%d\n", ms.data);
return 0;
}

None of my comments indicate any problem in the correctness of the
code.

 

[dandelion]

typedef struct my_struct_t my_struct_t;

<minor nitpick>
Does "struct my_struct_t" not suggest a type, which it isn't? Would naming
it "my_struct_s" not be better?
</minor nitpick>

 

[Jack Klein]

<minor nitpick>
Does "struct my_struct_t" not suggest a type, which it isn't? Would naming
it "my_struct_s" not be better?
</minor nitpick>

But struct my_struct_t IS a type. The keyword 'struct' is the only in
C that actually creates a type.

From the standard library there are div_t, ldiv_t, lldiv_t, and
imaxdiv_t, all of which are structures. And some other standard types
ending in "_t" are opaque enough that they may well be structure types
on at least some implementations.

 

[Lawrence Kirby]

On Thu, 11 Nov 2004 21:40:26 -0600, Jack Klein wrote:

....

But struct my_struct_t IS a type. The keyword 'struct' is the only in
C that actually creates a type.

There's always union. And the addition of qualifiers like const certainly
result in a different type.

Lawrence

 

[Malcolm]

Welcome back to the ng.