Arrays issue

[ataanis]

Hi,
I have the following statement, and i'm trying to understand why when
I do the second print it's giving me segmentation fault, both name[i]
and s_aliases[i] are char arrays e.g
THE FIRST PRINT WORKS FINE , which means that the aliases are in
s_aliases[i]

char names[NAMESIZE] and char s_aliases[2], h is a structure , that
contains s_aliases
defined as
struct servent *h and same for hostsev
while (h -> s_aliases[i] != NULL)
{
printf("Alias %d : \t%s\n",i, h->s_aliases[i]);
hostsev.names[i] = *h -> s_aliases[i];
i++;
} // loop thru aliases
printf("the first element in the hostsev aliases is %s \n",
hostsev.names[0]);
Can anybody tell me why?

[santosh]

at*****@gmail.com wrote:

Hi,
I have the following statement, and i'm trying to understand why when
I do the second print it's giving me segmentation fault, both name[i]
and s_aliases[i] are char arrays e.g
THE FIRST PRINT WORKS FINE , which means that the aliases are in
s_aliases[i]

char names[NAMESIZE] and char s_aliases[2], h is a structure , that
contains s_aliases
defined as
struct servent *h and same for hostsev
If you supply the actual declaration of your structures, then we can
provide better answers. Copy and paste them from the source code -
don't retype.
while (h -> s_aliases[i] != NULL)
{
printf("Alias %d : \t%s\n",i, h->s_aliases[i]);
hostsev.names[i] = *h -> s_aliases[i];

You cant simply assign on string to another like this. You'll have to
copy them character-by-character. Further I suspect the way your
accessing h->s_aliases[i] in the above expression is wrong.

[ataanis]

Santosh ,

struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;

struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;

struct servent *h;

while (h -> s_aliases[i] != NULL)
{
printf("Alias %d : \t%s\n",i, h->s_aliases[i]);
hostsev.names[i] = * h->s_aliases[i];
printf("the content of the structure %s \n",
hostsev.names[i]);
i++;
} // loop thru aliases

The first printf works , and the second doesn't which I believe means
that it's not storing
Thanks

[Richard Heathfield]

at*****@gmail.com said:

<snip>

struct servent *h;

while (h -> s_aliases[i] != NULL)

Stop right there. h is a pointer, but you haven't pointed it at any struct
servent object. Therefore its value is indeterminate, and any attempt to
dereference that value invokes undefined behaviour.

Point it somewhere useful.

<snip>

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

[Flash Gordon]

at*****@gmail.com wrote:

Hi,
I have the following statement, and i'm trying to understand why when
I do the second print it's giving me segmentation fault, both name[i]
and s_aliases[i] are char arrays e.g
THE FIRST PRINT WORKS FINE , which means that the aliases are in
s_aliases[i]

char names[NAMESIZE] and char s_aliases[2], h is a structure , that
contains s_aliases
defined as
struct servent *h and same for hostsev
Why go to all this effort to provide a hard to read description when you
could have simply posted some code?
while (h -> s_aliases[i] != NULL)
{
printf("Alias %d : \t%s\n",i, h->s_aliases[i]);
hostsev.names[i] = *h -> s_aliases[i];
i++;
} // loop thru aliases
printf("the first element in the hostsev aliases is %s \n",
hostsev.names[0]);
Can anybody tell me why?

Yes, you've got an error in your code. Try posting a small complete
compilable example of standard C code that shows your problem and
someone might be able to be more specific.
--
Flash Gordon, living in interesting times.
Web site - http://home.flash-gordon.me.uk/
comp.lang.c posting guidelines and intro:
http://clc-wiki.net/wiki/Intro_to_clc

Inviato da X-Privat.Org - Registrazione gratuita http://www.x-privat.org/join.php

[ataanis]

Santosh , the message was supposed to be

struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;
struct servent {
char *s_name; /* official service name
*/
char **s_aliases; /* alias list */
int s_port; /* port number */
char *s_proto; /* protocol to use */
}
struct servent *h;
while (h -> s_aliases[i] != NULL)
{
printf("Alias %d : \t%s\n",i,
h->s_aliases[i]);
hostsev.names[i] = * h->s_aliases[i];

printf("the content of the structure %s
\n",
hostsev.names[i]);
i++;
} // loop thru aliases
The first printf works , and the second doesn't which I believe means
that it's not storing
Thanks

[santosh]

[e-mail] wrote:

Santosh , the message was supposed to be

struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;
struct servent {
char *s_name; /* official service name
*/
char **s_aliases; /* alias list */
int s_port; /* port number */
char *s_proto; /* protocol to use */
}
struct servent *h;
while (h -> s_aliases[i] != NULL)
As Richard says, you haven't set h to point to any instance of servant
structure.
{
printf("Alias %d : \t%s\n",i,
h->s_aliases[i]);
hostsev.names[i] = * h->s_aliases[i];
Are you sure that NAMESIZE will be sufficient here? If not then data
might get overwritten. Also you've to do a element wise copy here.

printf("the content of the structure %s
\n",
hostsev.names[i]);
i++;
} // loop thru aliases
The first printf works , and the second doesn't which I believe means
that it's not storing

Yes, you're not copying the strings at all. You need another loop to do
that. Something like:

for(cnt = 0; h->s_aliases[i][cnt] != '\0'; ++cnt)
hostsev.names[cnt] = h->s_aliases[i][cnt];

[ataanis]

I respect your answer, but if that's the case then how come I get the
following output ?

Alias 0 : postoffice
Alias 1 : pop-2
Segmentation fault
That means that the first print is working and the second is not
Thanks

[ataanis]

Santosh , the message was supposed to be

struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;
struct servent {
char *s_name; /* official service name
*/
char **s_aliases; /* alias list */
int s_port; /* port number */
char *s_proto; /* protocol to use */
}
struct servent *h;
while (h -> s_aliases[i] != NULL)
{
printf("Alias %d : \t%s\n",i,
h->s_aliases[i]);
hostsev.names[i] = * h->s_aliases[i];

printf("the content of the structure %s

\n",
hostsev.names[i]);
i++;
} // loop thru aliases
The first printf works , and the second doesn't which I believe means
that it's not storing
Thanks

[ataanis]

is there any specific reason why you made the array 2 dimensional ?

[Richard Heathfield]

at*****@gmail.com said:

I respect your answer, but if that's the case then how come I get the
following output ?

Alias 0 : postoffice
Alias 1 : pop-2
Segmentation fault
That means that the first print is working and the second is not

I jumped off a cliff with no safety net, and hit a few rocks on the way
down. I didn't sustain any serious injuries on hitting the first rock, but
on the second I broke my leg, so obviously jumping off wasn't a mistake - I
must have done something wrong after the first bounce.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

[ataanis]

Richard,
ok i'm really sorry but I believe this is the statement I forgot to
mention in my previous posts.

h = getservbyname(name,protocol)

[ataanis]

can you tell me why you made the array 2 dimensional ?

[Richard Heathfield]

at*****@gmail.com said:

Richard,
ok i'm really sorry but I believe this is the statement I forgot to
mention in my previous posts.

h = getservbyname(name,protocol)

One cannot help but wonder what other highly relevant information has been
omitted.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

[ataanis]

Richard,
Absolutely nothing else was forgotten
thanks

[ataanis]

so can you tell me why you made a 2 dimensional array ?
thanks

[Keith Thompson]

at*****@gmail.com writes:

Santosh , the message was supposed to be

struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;
struct servent {
char *s_name; /* official service name
*/
char **s_aliases; /* alias list */
int s_port; /* port number */
char *s_proto; /* protocol to use */
}
struct servent *h;

[snip]

Ok, stop right there.

The above code will not compile. There's a missing semicolon after
the declaration of "struct servent".

We have no way of knowing what other differences there are between the
code you posted and the code that's actually causing problems. The
actual problem could well be in something that you didn't post. We're
not going to waste our time trying to guess what the code *really*
looks like.

Post a small, complete, compilable program that illustrates the
problem. Don't try to re-type it; copy-and-paste it directly from a
source file that you've actually compiled. Don't post a single line
of code to be added to what you've already posted; post a complete
program.

And please read <http://cfaj.freeshell.org/google/> and
<http://clc-wiki.net/wiki/Introduction_to_comp.lang.c>.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Barry Schwarz]

On 26 May 2006 14:05:06 -0700, at*****@gmail.com wrote:

Santosh ,

struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;

struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;
You declare the struct service twice. You define the object hostsev
twice.

struct servent *h;
There is no struct servent.

while (h -> s_aliases[i] != NULL)
{
printf("Alias %d : \t%s\n",i, h->s_aliases[i]);
hostsev.names[i] = * h->s_aliases[i];
printf("the content of the structure %s \n",
hostsev.names[i]);
i++;
} // loop thru aliases

The first printf works , and the second doesn't which I believe means
that it's not storing
Thanks

In spite of your claim else thread to have forgotten nothing, you have
forgotten to show us your actual code.
Remove del for email

[ataanis]

Alright , Please accept my sincere appologies, here is the complete
code of my server side, I didn't want to post the whole code thinking
that it would be too much for people to read
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/select.h>
#include <string.h>
#include <netdb.h>
#define MYPORT 53491 /* the port users will be connecting to */

#define MSGSIZE 128

#define NAMESIZE 2

static int read_from_client (int fd, struct sockaddr_in *address);

/**
Accepts a string on the command line, sends that string to any
client that connects.
*/
int main (int argc, char *argv[])
{
char *message = "Waiting for another request";
int sockfd; /* datagram socket (2-way) */
struct sockaddr_in my_addr; /* my address */
int addr_size;
int enable = 1;
struct servent *h;
int port;
struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;

/* --- Check args and set up the message --- */
if (argc > 2) { /* usage message */
fprintf(stderr, "Usage: myserver port\n");
exit(1);
}
if (argc == 2) { /* take message from command line */
message = argv[1];
}

/* --- Create the socket --- */
if ((sockfd = socket(AF_INET, SOCK_DGRAM, 0)) == -1) {
perror("Can't create socket");
exit(1);
}

/* --- Bind the socket, making its address reusable --- */
my_addr.sin_family = AF_INET;
my_addr.sin_port = htons(MYPORT);
my_addr.sin_addr.s_addr = htonl(INADDR_ANY);
/* Note the use of INADDR_ANY to get the OS to use whatever local
interfaces are found. */

if (setsockopt(sockfd,
SOL_SOCKET, SO_REUSEADDR,
&enable, sizeof(int)) == -1) {
perror("Can't set socket option");
exit(1);
}

if (bind(sockfd,
(struct sockaddr *)&my_addr,
sizeof(my_addr)) == -1) {
perror("Could not bind");
exit(1);
}

/* --- No listen or accept code here - just wait for clients to make
requests --- */

/* --- Accept datagrams and serve our message --- */
while (1) {
struct sockaddr client_addr; /* client's address */
socklen_t addr_size = sizeof(struct sockaddr);
char client_buffer[MSGSIZE];
int readsz = 0;
char *name;
char *message1;
char *message2;
char *port_s;
char *protocol;
char *protocol1;
char **aliases;
//char *alias1;
int x = 1;
int i = 0;
int cnt;
char *SeqNumber;

/* Read from the client, get the client address filled in. Then we
can respond back to the same client. */
// memset(&client_addr, '\0', sizeof(struct sockaddr));
if ((readsz = recvfrom(sockfd, client_buffer, MSGSIZE, 0,&client_addr,
&addr_size)) > 0)
{
/* cast the client address to internet format for convenience */
struct sockaddr_in *in_addr = (struct sockaddr_in *)&client_addr;
client_buffer[readsz] = '\0';
/* take apart the address and see what we have, then print the
message */
fprintf(stderr, "Client: family=%d, port=%d, addr=%s,
length=%d\n",client_addr.sa_family,ntohs(in_addr->sin_port),inet_ntoa(in_addr->sin_addr),addr_size);

SeqNumber = strtok (client_buffer, ",");

name = strtok (NULL,",");
hostsev.sequence = atoi(SeqNumber);
printf(" there %i \n", hostsev.sequence);
while (1)
{
/* extract protocol from string sequence */
protocol = strtok(NULL,"\n");
fprintf(stderr,"Your request is the following: SeqNumber: %s, Name Of
Service: %s, Protocol: %s \n",SeqNumber,name,protocol);
//h = getservbyname(name,protocol);
if ((h = getservbyname(name,protocol)) == NULL)
{
printf("%s: unknown host %s \n",name,protocol);
exit(0);
}
else
{
printf("The port Number for your request is %d \n",ntohs(h
->s_port));
hostsev.port = h -> s_port;
printf("here %i \n" , ntohs(hostsev.port));

//port_s = atoi(itoa(ntohs(h -> s_port));
//exit(0);
i = 0;
hostsev.names[NAMESIZE] = 0 ;
while (h -> s_aliases[i] != NULL)
{
//printf("Alias %d : \t%s\n",i, h->s_aliases[i]);

for(cnt = 0; h->s_aliases[i] != '\0'; ++cnt)
hostsev.names[cnt] = *h->s_aliases[i];
//hostsev.names[i] += *h->s_aliases[i];
//printf("the content of the structure %s \n", hostsev.names[i]);
i++;
} // loop thru aliases
break;
}
}

printf("the content of the structure %s \n", hostsev.names[0]);
/* Next, we write our message and close the socket. */
if (sendto(sockfd, &hostsev, sizeof(hostsev), 0,&client_addr,
addr_size) == -1)
{
perror("Failed to write to client");
}
else
{
fprintf(stderr, "%s\n", message);
}
}

else {
printf("Failed to read from client\n");
}
}

return 0;
}
Thank you

[CBFalconer]

at*****@gmail.com wrote:

Alright , Please accept my sincere appologies, here is the complete
code of my server side, I didn't want to post the whole code thinking
that it would be too much for people to read

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/select.h>
#include <string.h>
#include <netdb.h>

.... snip remaining non-standard code ...

Of the above, only stdio, stdlib, errno, and string are standard
include files. The rest are unknown quantities. This newsgroup
deals only with standard C, so you should adjust your code
accordingly before posting here, or alternatively go to a newsgroup
that deals with your system.

In addition, without context the whole thing is pointless. I would
assume that something doesn't work. There is no indication what.

In general on usenet you should realize that readers may very well
not have convenient access to previous articles in a thread. That
means that your reply articles should include adequate context, so
that they stand by themselves. Google is NOT usenet, it is only a
very poor interface to the real usenet system. To include proper
context when using google, see my sig. below. Please be sure to
read the referenced URLs.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>
Also see <http://www.safalra.com/special/googlegroupsreply/>

[Andrew Poelstra]

On 2006-05-26, at*****@gmail.com <at*****@gmail.com> wrote:

Richard,
Absolutely nothing else was forgotten
thanks

First rule of C programming: Heed _everything_ that Richard
Heathfield says.

--
Andrew Poelstra < http://www.wpsoftware.net/blog >
To email me, use "apoelstra" at the above address.
It's just like stealing teeth from a baby.

[Barry Schwarz]

On 27 May 2006 00:19:28 -0700, at*****@gmail.com wrote:

Alright , Please accept my sincere appologies, here is the complete
code of my server side, I didn't want to post the whole code thinking
that it would be too much for people to read
No you need to tell us which printf you think generates a segmentation
fault.

Since you are using a bunch of non-standard structures and functions,
it would help to have the declarations of those also.
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/select.h>
#include <string.h>
#include <netdb.h>
#define MYPORT 53491 /* the port users will be connecting to */

#define MSGSIZE 128

#define NAMESIZE 2

static int read_from_client (int fd, struct sockaddr_in *address);

/**
Accepts a string on the command line, sends that string to any
client that connects.
*/
int main (int argc, char *argv[])
{
char *message = "Waiting for another request";
int sockfd; /* datagram socket (2-way) */
struct sockaddr_in my_addr; /* my address */
int addr_size;
int enable = 1;
struct servent *h;
int port;
struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;

/* --- Check args and set up the message --- */
if (argc > 2) { /* usage message */
fprintf(stderr, "Usage: myserver port\n");
exit(1);
}
if (argc == 2) { /* take message from command line */
message = argv[1];
}

/* --- Create the socket --- */
if ((sockfd = socket(AF_INET, SOCK_DGRAM, 0)) == -1) {
perror("Can't create socket");
exit(1);
}

/* --- Bind the socket, making its address reusable --- */
my_addr.sin_family = AF_INET;
my_addr.sin_port = htons(MYPORT);
my_addr.sin_addr.s_addr = htonl(INADDR_ANY);
/* Note the use of INADDR_ANY to get the OS to use whatever local
interfaces are found. */

if (setsockopt(sockfd,
SOL_SOCKET, SO_REUSEADDR,
&enable, sizeof(int)) == -1) {
perror("Can't set socket option");
exit(1);
}

if (bind(sockfd,
(struct sockaddr *)&my_addr,
my_addr is a struct sockaddr_in. Here you cast its address to struct
sockaddr *. Are you ABSOLUTELY sure that my_addr and each of its
members is properly aligned to be treated as a struct sockaddr? If
not, this could invoke undefined behavior either here in the code that
generates the argument or somewhere in bind() where the argument is
used.
sizeof(my_addr)) == -1) {
perror("Could not bind");
exit(1);
}

/* --- No listen or accept code here - just wait for clients to make
requests --- */

/* --- Accept datagrams and serve our message --- */
while (1) {
struct sockaddr client_addr; /* client's address */
socklen_t addr_size = sizeof(struct sockaddr);
char client_buffer[MSGSIZE];
int readsz = 0;
char *name;
char *message1;
char *message2;
char *port_s;
char *protocol;
char *protocol1;
char **aliases;
//char *alias1;
int x = 1;
int i = 0;
int cnt;
char *SeqNumber;

/* Read from the client, get the client address filled in. Then we
can respond back to the same client. */
// memset(&client_addr, '\0', sizeof(struct sockaddr));
Is all bits zero a valid value for every possible member of
client_addr? If not, this could possibly cause undefined behavior in
recvfrom.
if ((readsz = recvfrom(sockfd, client_buffer, MSGSIZE, 0,&client_addr,
&addr_size)) > 0)
{
/* cast the client address to internet format for convenience */
struct sockaddr_in *in_addr = (struct sockaddr_in *)&client_addr;
Another potential alignment problem, this time in the opposite
direction.
client_buffer[readsz] = '\0';
/* take apart the address and see what we have, then print the
message */
fprintf(stderr, "Client: family=%d, port=%d, addr=%s,
length=%d\n",client_addr.sa_family,ntohs(in_add r->sin_port),inet_ntoa(in_addr->sin_addr),addr_size);
Do the first three arguments after the format string truly match the
types implied by the conversion specifications, int, int, and char*?

SeqNumber = strtok (client_buffer, ",");

name = strtok (NULL,",");
hostsev.sequence = atoi(SeqNumber);
printf(" there %i \n", hostsev.sequence);
while (1)
{
/* extract protocol from string sequence */
protocol = strtok(NULL,"\n");
fprintf(stderr,"Your request is the following: SeqNumber: %s, Name Of
Service: %s, Protocol: %s \n",SeqNumber,name,protocol);
//h = getservbyname(name,protocol);
if ((h = getservbyname(name,protocol)) == NULL)
{
printf("%s: unknown host %s \n",name,protocol);
exit(0);
}
else
{
printf("The port Number for your request is %d \n",ntohs(h
->s_port));
hostsev.port = h -> s_port;
printf("here %i \n" , ntohs(hostsev.port));

//port_s = atoi(itoa(ntohs(h -> s_port));
//exit(0);
i = 0;
hostsev.names[NAMESIZE] = 0 ;
while (h -> s_aliases[i] != NULL)
{
//printf("Alias %d : \t%s\n",i, h->s_aliases[i]);

for(cnt = 0; h->s_aliases[i] != '\0'; ++cnt)
hostsev.names[cnt] = *h->s_aliases[i];
//hostsev.names[i] += *h->s_aliases[i];
//printf("the content of the structure %s \n", hostsev.names[i]);
i++;
} // loop thru aliases
break;
}
}

printf("the content of the structure %s \n", hostsev.names[0]);
/* Next, we write our message and close the socket. */
if (sendto(sockfd, &hostsev, sizeof(hostsev), 0,&client_addr,
addr_size) == -1)
{
perror("Failed to write to client");
}
else
{
fprintf(stderr, "%s\n", message);
}
}

else {
printf("Failed to read from client\n");
}
}

return 0;
}
Thank you

Remove del for email

[ataanis]

Barry,

The only printf that isn't working and causing the segmentation fault
is the following
printf("the content of the structure %s \n", hostsev.names[0]);

Thanks

[Frank Silvermann]

Andrew Poelstra wrote:

On 2006-05-26, at*****@gmail.com <at*****@gmail.com> wrote:

Richard,
Absolutely nothing else was forgotten
thanks

First rule of C programming: Heed _everything_ that Richard
Heathfield says.

As this thread bleeds on with non-Standard slop, I am quite confident
that heeding everything that Mr. Heathfield says is an impossibly-high
standard for this young man, in particular when clc's most prodigious
contributor has already pointed out exactly where to start fixing
things. I, however, disagree with Mr. Heathfield. I think the original
mistake is not spelling 'servant' correctly, although this is not a
problem in C syntax, rather a grating error in communication generally.
frank

[santosh]

at*****@gmail.com wrote:

Barry,

The only printf that isn't working and causing the segmentation fault
is the following
printf("the content of the structure %s \n", hostsev.names[0]);

Which could be because of invoking undefined behaviour, (as Barry has
pointed out), at multiple places. When you cause undefined behaviour,
anything could happen, which in this case could be printf() failing. If
even after you rectify the undefined constructs, the same error occurs,
then we can see about what to do.

[ataanis]

That is why I pointed to the while loop right before it e,g. :
while (h -> s_aliases[i] != NULL)
{
//printf("Alias %d : \t%s\n",i,
h->s_aliases[i]);

for(cnt = 0; h->s_aliases[i] != '\0';
++cnt)
hostsev.names[cnt] = *h->s_aliases[i];
//hostsev.names[i] += *h->s_aliases[i];
//printf("the content of the structure
%s \n", hostsev.names[i]);
i++;
} // loop thru aliases
break;
}
printf("the content of the structure %s \n", hostsev.names[0]);
This is the printf that isn't working

[Keith Thompson]

at*****@gmail.com writes:

Barry,

The only printf that isn't working and causing the segmentation fault
is the following
printf("the content of the structure %s \n", hostsev.names[0]);

I've already pointed you to <http://cfaj.freeshell.org/google/>.

Read it. Read it now.

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Clever Monkey]

Frank Silvermann wrote:

Andrew Poelstra wrote:

On 2006-05-26, at*****@gmail.com <at*****@gmail.com> wrote:

Richard, Absolutely nothing else was forgotten
thanks

First rule of C programming: Heed _everything_ that Richard
Heathfield says.

As this thread bleeds on with non-Standard slop, I am quite confident
that heeding everything that Mr. Heathfield says is an impossibly-high
standard for this young man, in particular when clc's most prodigious
contributor has already pointed out exactly where to start fixing
things. I, however, disagree with Mr. Heathfield. I think the original
mistake is not spelling 'servant' correctly, although this is not a
problem in C syntax, rather a grating error in communication generally.
frank

I can't tell if you are making a very dry joke. If so, the joke's on me!

[Chris Torek]

In article <11*********************@j73g2000cwa.googlegroups. com>
<at*****@gmail.com> wrote:

Alright , Please accept my sincere appologies, here is the complete
code of my server side, I didn't want to post the whole code thinking
that it would be too much for people to read
It is generally wise to shrink the program down (by removing
parts of it that are working) until you have a small "nugget"
of failing code, and then post that. Of course, in the process
of doing that you often find the problem yourself, but you could
consider that a bonus. :-)

[various #includes snipped]
#include <sys/types.h>
#include <sys/socket.h> [etc]

Many of these headers are non-Standard (as in "not ANSI/ISO Standard
C", which is pretty limited) headers. Unfortunately at least a
few of them are required to demonstrate a remaining problem after
fixing the first one. Still, onward:
int main (int argc, char *argv[])
{
[snippage]
struct service {
int sequence;
int port;
char names[NAMESIZE];
} hostsev;
Note that hostsev.names is an array (of NAMESIZE elements) of
type "char". Thus, it can hold at most NAMESIZE "char"s, which
could be a sequence of "char"s like 'x', 'y', 'z'; if such a
sequence of "char"s ends with a '\0' character, it is a (single)
valid string that could be printed with "%s".
struct servent *h; [snippage] char **aliases;
//char *alias1;
"//"-comments are invalid in C89/C90, but are valid in C99. They
do have the drawback of often failing to survive line-wrapping in
postings. When I fed this code to "gcc -ansi" (same as -std=c89)
I got a pile of errors due to the invalid (for C89) //-comments.

I then fed the code to gcc without requesting C89 ("gcc -Wall -O -c"),
and I still got a pile of warnings:

x.c: In function `main':
x.c:158: warning: format argument is not a pointer (arg 2)
x.c:97: warning: unused variable `x'
x.c:95: warning: unused variable `aliases'
x.c:94: warning: unused variable `protocol1'
x.c:92: warning: unused variable `port_s'
x.c:91: warning: unused variable `message2'
x.c:90: warning: unused variable `message1'
x.c:35: warning: unused variable `port'
x.c:32: warning: unused variable `addr_size'
x.c: At top level:
x.c:21: warning: `read_from_client' declared `static' but never defined

The warning at line 158 is perhaps the most significant. Here is
some of the code leading to line 158, plus line 158 itself (without
lines that use //-comments to comment out all but whitespace). I
have adjusted indentation somewhat for posting purposes.

Note that I am going to highlight both tactical errors (mistakes
in individual steps of execution) and strategic errors (mistakes
in the overall design / plan of the program). There are quite a
few of the latter, but the former are actually bigger problems.
They are not causing the crashes you see, but require more work
to fix, eventually.
i = 0;
hostsev.names[NAMESIZE] = 0 ;
The object "hostsev.names" is, as I stated earlier, an array (of
size NAMESIZE) of "char". The valid subscripts for this array go
from 0 to NAMESIZE-1 inclusive. Setting hostsev.names[NAMESIZE]
to 0 causes undefined behavior. This could crash your program at
this point, but usually the effect of the undefined behavior is
to keep going, while maybe messing up something else later.
while (h -> s_aliases[i] != NULL) {
for (cnt = 0; h->s_aliases[i] != '\0'; ++cnt)
hostsev.names[cnt] = *h->s_aliases[i];
i++;
}
This nested pair of loops is very badly wrong, and if you ever
encountered a service with extra aliases, it would probably crash
your program even before line 158.

Here "h->s_aliases" comes from one of those non-Standard headers.
Luckily I happen to know what is in it (because I was involved in
some of the projects that created those headers, back in the 1980s).
It happens to be an object of type "pointer to pointer to char"
(or "char **"), which points to the first of one or more
valid "pointer to char"s, with the last of those "pointer to char"s
set to NULL. Any "pointer to char"s before that last one are
valid pointers that point to the first of one or more "char"s,
with the last of those "char"s in turn being a '\0'.

Pictorially, this comes out to, for instance:

h->s_aliases (unnamed block of pointers)
+-----+ +-----+
| *--|------> | *--|-> {'a', 'b', 'c', '\0' }
+-----+ +-----+
| *--|---> {'d', 'e', 'f', 'g', 'h', '\0' }
+-----+
| *--|--------> {'i', 'j', '\0' }
+-----+
| NULL|
+-----+

(if "h" happens to point to a service with three aliases "abc",
"defgh", and "ij").

The "while" loop -- which stylistically might be better written
as a "for" loop -- runs through the non-NULL aliases. If you
had written, for instance:

for (i = 0; h->s_aliases[i] != NULL; i++)
printf("%s\n", h->s_aliases[i]);

this would print "abc\n", "defgh\n", and "ij\n" respectively for
the three non-NULL pointers in the illustration above. But this
is not what appears inside the loop. Instead, you refer (in an
inner "for" loop) to *h->aliases[i]. The binding on this is such
that it means the same as *(h->aliases[i]), which is in turn the
same as h->alisaes[i][0]. What you have written can be re-expressed
thus:

for (i = 0; h->s_aliases[i] != NULL; i++)
for (cnt = 0; h->s_aliases[i] != '\0'; cnt++)
hostsev.names[cnt] = h->s_aliases[i][0];

The second line here is quite suspect: you test against '\0', but
h->s_aliases[i] is a pointer, which you (earlier, correctly) test
against NULL. In C, any integral constant zero is suitable for use
as a NULL pointer, including '\0', which is just another way to write
zero -- so this pair of loops can be re-re-expressed as:

for (i = 0; h->s_aliases[i] != NULL; i++)
for (cnt = 0; h->s_aliases[i] != NULL; cnt++)
hostsev.names[cnt] = h->s_aliases[i][0];

Now the problem is more obvious: if the outer loop runs at all,
h->s_aliases[i] is definitely not equal to NULL. The inner loop
then tests h->s_aliases[i] and runs until it is equal to NULL,
which can never occur (barring "undefined behavior" of course).
Clearly you *meant* to test h->s_aliases[i][cnt], not
h->s_aliases[i]. But that would lead to:

for (i = 0; h->s_aliases[i] != NULL; i++)
for (cnt = 0; h->s_aliases[i][cnt] != '\0'; cnt++)
hostsev.names[cnt] = h->s_aliases[i][0];

and again there is an obvious problem: the inner loop now tests
h->s_aliases[i][cnt], but copies h->s_aliases[i][0].

It has a second, slightly more subtle problem: it fails to
copy the '\0' character that terminates a C string. Presumably
the earlier assignment to the non-existent hostsev.names[NAMESIZE]
element was supposed to handle that, but this method is
hopelessly flawed: not only does it overwrite a non-existent
element, it also does not put the '\0' in the right place.

Fixing both of these is easy enough -- use strcpy() to copy
strings:

for (i = 0; h->s_aliases[i] != NULL; i++)
strcpy(hostsev.names, h->s_aliases[i]);

This still has potential bad behavior if the length of any of the
aliases is longer than fits into hostsev.names -- and of course it
illustrates the strategic error: there are, at least potentially,
a large number of aliases, each of which needs some number of
"char"s to store. The actual number needed, for any given alias,
is strlen(h->s_aliases[i])+1, where the +1 accounts for the
terminating '\0'. If there are (say) 47 aliases and each one
has an average storage requirement of 25 "char"s, you will need
25 * 47 = 1175 "char"s to hold them all. You will also need to
remember (or find) where the 47 names start -- but you have only
one "array N of char", so even if N is big enough (1175) you still
have to solve that, *if* you really want to keep all these aliases
around.

Luckily (?), h->s_aliases[i] contains only *additional* aliases
for any given service. The *primary* service name is in h->s_name.
In most cases, h->s_alises[0] is NULL, so that the outer loop does
not run at all.

Finally, if luck (whether it is good or bad) holds out, we actually
reach line 158, which reads:
printf("the content of the structure %s \n", hostsev.names[0]);

But hostsev.names[0] is a single "char". The "%s" format, in
printf, requires a valid value of type "char *", pointing to the
first of zero or more non-'\0' characters, terminated by a '\0'
character. It then prints each of the non-'\0' characters, as
if you had done:

for (i = 0; valid_ptr[i] != '\0'; i++)
putchar(valid_ptr[i]);

If you take a single "char" and coerce it into a pointer, the
resulting value is unlikely to be valid, so that "valid_ptr[i]"
crashes even when i is zero.
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) [phone]
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.

[Chris Torek]

In article <e5*********@news4.newsguy.com> I wrote, in part:

... I am going to highlight both tactical errors (mistakes
in individual steps of execution) and strategic errors (mistakes
in the overall design / plan of the program). There are quite a
few of the latter, but the former are actually bigger problems.

Oops, I reversed the "former" and "latter" somehow. Strategic
errors usually require more work to fix than do tactical errors.
(Of course, either one can take out the entire program.)

(Other simple typos -- there was at least one -- are left as an
exercise to the reader. :-) )
--
In-Real-Life: Chris Torek, Wind River Systems
Salt Lake City, UT, USA (40°39.22'N, 111°50.29'W) [phone]
email: forget about it http://web.torek.net/torek/index.html
Reading email is like searching for food in the garbage, thanks to spammers.