some misunderstanding of pointers

[Roman Mashak]

Hello, All!

Short piece of code:

....
struct in_addr {
unsigned long int s_addr;
};

struct iphdr {
unsigned char ihl:4, version:4;
unsigned char tos;
short unsigned int tot_len;
short unsigned int id;
short unsigned int frag_off;
unsigned char ttl;
unsigned char protocol;
short unsigned int check;
unsigned int saddr;
unsigned int daddr;
};

extern char *inet_ntoa(struct in_addr);
struct iphdr *ip;
....
/* somewhere here 'ip' is filled in with values */
....
printf("%-15s ", inet_ntoa(ip->saddr));

Compiling with "gcc -Wall -g -ansi -pedantic" result with a warning: "
incompatible type for argument 1 of `inet_ntoa' ". I came to know the
solution is to change the call into:

printf("%-15s ", inet_ntoa(*(struct in_addr *)&ip->saddr));

But I can't entirely understand this construction - the conglomeration of
pointers and '&' operators, could you please explain it to me.

PS. It's not homework.

With best regards, Roman Mashak. E-mail: mr*@tusur.ru

[Suman]

Roman Mashak wrote:

[ ...]

struct in_addr {
unsigned long int s_addr;
};

struct iphdr {
[snip members]
.... unsigned int saddr;
This is an unsigned integer.
unsigned int daddr;
};

extern char *inet_ntoa(struct in_addr);
And this expects a struct in_addr type *object* (term used with
caution),
to be passed to it.
struct iphdr *ip;
...
/* somewhere here 'ip' is filled in with values */
/* provided ip was allocated in the first place! */
...
printf("%-15s ", inet_ntoa(ip->saddr));
You are now passing an unsigned int instead of a struct in_addr.
Thus spake The Compiler:
incompatible type for argument 1 of `inet_ntoa' ". I came to know the
solution is to change the call into:

printf("%-15s ", inet_ntoa(*(struct in_addr *)&ip->saddr));
Coerce the compiler.
But I can't entirely understand this construction - the conglomeration of
pointers and '&' operators, could you please explain it to me.

ip->saddr is an unsigned int.
&ip->saddr is a pointer an unsigned int.
(struct in_addr *)&ip->saddr makes the compiler treat
the above as a pointer
to struct in_addr
*(struct in_addr *)&ip->saddr if possible, which it is, in *this*
case,
dereference the pointer to get an
object of
type struct in_addr
Luckily for you the struct in_addr
has only one member, and that is an unsinged int, so casting here
should not be a problem. However , think of a more general case,
and it would have been better if you had embedded a member in
the struct iphdr of type struct in_addr as follows:

struct iphdr {
....
/*unsigned int saddr;*/
struct in_addr sin_addr;
};
and done something like:

printf("%-15s ", inet_ntoa(ip->sin_addr));

This would of come handy were you change struct in_addr in future.
But that wouldn't have required changing struct iphdr.
HTH.

[slebetman]

Roman Mashak wrote:

Hello, All!

Short piece of code:

...
struct in_addr {
unsigned long int s_addr;
};

struct iphdr {
unsigned char ihl:4, version:4;
unsigned char tos;
short unsigned int tot_len;
short unsigned int id;
short unsigned int frag_off;
unsigned char ttl;
unsigned char protocol;
short unsigned int check;
unsigned int saddr;
unsigned int daddr;
};

extern char *inet_ntoa(struct in_addr);
struct iphdr *ip;
...
/* somewhere here 'ip' is filled in with values */
...
printf("%-15s ", inet_ntoa(ip->saddr));

Compiling with "gcc -Wall -g -ansi -pedantic" result with a warning: "
incompatible type for argument 1 of `inet_ntoa' ". I came to know the
solution is to change the call into:

printf("%-15s ", inet_ntoa(*(struct in_addr *)&ip->saddr));

But I can't entirely understand this construction - the conglomeration of
pointers and '&' operators, could you please explain it to me.

PS. It's not homework.

With best regards, Roman Mashak. E-mail: mr*@tusur.ru

The function is:

char *inet_ntoa(struct in_addr address);

Meaning: Function inet_ntoa, accepts an address parameter of type
"struct in_addr", and returns a pointer to data of type char.

Don't be fooled by the prototype. The "in_addr" is not a parameter name
but a type. Properly, it is "struct in_addr".

OK, so you are passing it:

inet_ntoa(*(struct in_addr *)&ip->saddr);

Meaning:

* = the data of
(struct in_addr *) = cast to a pointer to struct in_addr
& = the address of
ip->saddr = the integer saddr pointed to from ip

Now, re-arrange the above into English:

The data of, the address of the integer saddr in ip, cast to a pointer
of struct in_addr.

This is highly confusing. If I'm not mistaken, the code:

printf("%-15s ", inet_ntoa((struct in_addr)ip->saddr));

is what you really want but I'm guessing the compiler complained about
casting an int into a struct. Which is why you had to do the convoluted
thing you did.

An easier to understand code is:

extern char *inet_ntoa(struct in_addr);
struct iphdr *ip;
struct in_addr temp_addr;
...
/* somewhere here 'ip' is filled in with values */
...
temp_addr.s_addr = ip->saddr;
printf("%-15s ", inet_ntoa(temp_addr));
Just give the function what it wants and don't be too clever. The extra
temp variable is just 32 bits (typically), you're not saving that much
memory with your original method.

[Roman Mashak]

RM> Short piece of code:

RM> ...
RM> struct in_addr {

Thanks guys a lot for precious explanations!

With best regards, Roman Mashak. E-mail: mr*@tusur.ru

[slebetman]

Roman Mashak wrote:

RM> Short piece of code:

RM> ...
RM> struct in_addr {

Thanks guys a lot for precious explanations!

With best regards, Roman Mashak. E-mail: mr*@tusur.ru

No problem, glad to help..

[Simon Biber]

Roman Mashak wrote:

...
struct in_addr {
unsigned long int s_addr;
};

struct iphdr {
unsigned char ihl:4, version:4;
unsigned char tos;
short unsigned int tot_len;
short unsigned int id;
short unsigned int frag_off;
unsigned char ttl;
unsigned char protocol;
short unsigned int check;
unsigned int saddr;
unsigned int daddr;
};

extern char *inet_ntoa(struct in_addr);
struct iphdr *ip;
...
/* somewhere here 'ip' is filled in with values */
...
printf("%-15s ", inet_ntoa(ip->saddr));

Compiling with "gcc -Wall -g -ansi -pedantic" result with a warning: "
incompatible type for argument 1 of `inet_ntoa' ". I came to know the
solution is to change the call into:

printf("%-15s ", inet_ntoa(*(struct in_addr *)&ip->saddr));

This makes the computer read from the memory allocated to ip->saddr, as
if it were actually of type struct in_addr.

There is a serious problem lurking here, which is when int and long have
a different size. On today's 32-bit computers, they are typically both
32 bits. However, on 64-bit computers, it is common for long to be 64
bits while int is still 32 bits. This will cause the computer to read an
extra 4 bytes after saddr, effectively the value of daddr, and combine
the two values together to produce a single huge value.

Here's a better solution, using a temporary variable:

struct in_addr address;
address.s_addr = ip->saddr;
printf("%-15s ", inet_ntoa(address);

Here the conversion from unsigned int to unsigned long is done
correctly, preserving the value instead of possibly reading extra memory
that it shouldn't read.

--
Simon.