------------------------------------- Programming suggestions for Lab #01 ------------------------------------- -Ian! D. Allen - idallen@idallen.ca Here are some comments that will help you fix your code to work correctly and earn full marks in future assignments. - missing comments on the code you added - if you write code - comment it (see notes file programming_style.txt) - future assignments: no comments, no marks - use the exact file names given in the lab exercise - if the lab says to use a particular set of names to the cstsubmit command, you have to use those names or else my marking-assist scripts won't find your files for marking - don't write useless comments (see notes file programming_style.txt) - also: incorrect spelling leaves a bad impression 97 /* Ignors the child signal */ 98 signal (SIGCHLD, SIG_IGN); - always exit on a fatal error; don't keep going on in the code - give an indication of what the error was (perhaps using errno or perror()) 67 if(listen(sockfd,5) == -1){ 68 fprintf(stderr,"ERROR on return from listen\n"); 69 } - errno and perror() only apply to failed system calls; don't use otherwise - errno and perror() use is undefined after a *successful* system call - thus: don't do anything major between a failed system call and perror() 100 printf("Client has disconnected\n"); 101 if (n < 0) error("ERROR reading from socket"); - avoid duplicate and inconsistent coding - perror() vs. error() - use one or the other; not both - avoid inconsistent indentation style - correct indentation is essential; unreadable code won't be marked - Unix tab stops are every 8 spaces; your submitted code must match 58 if (setsockopt(sockfd,SOL_SOCKET,SO_REUSEADDR,&yes,sizeof(int)) == -1) { 59 perror("setsockopt"); 60 exit(1); 61 } 62 if (bind(sockfd, (struct sockaddr *) &serv_addr, 63 sizeof(serv_addr)) < 0) 64 error("ERROR on binding"); - don't test unsigned numbers for negative values XX unsigned int sockfd = socket( ... XX if ( sockfd < 0 ) .... - don't place code after an infinite while(1) loop 74 while (1) { 76 newsockfd = accept(sockfd, 77 (struct sockaddr *) &cli_addr, (socklen_t *) &clilen); 79 if (newsockfd < 0) 80 error("ERROR on accept"); 82 pid = fork(); 84 if (pid < 0) 85 error("ERROR on fork"); 87 if (pid == 0) { 89 close(sockfd); 90 dostuff(newsockfd); 91 exit(0); 92 } 93 else 94 close(newsockfd); 95 } /* end of while */ 98 signal (SIGCHLD, SIG_IGN); 101 alarm(30*60); 103 return 0; /* we never get here */ - don't use casting to remove argument passing errors (fix the declaration) XX char clilen; /* very wrong; but cast hides it */ 76 newsockfd = accept(sockfd, 77 (struct sockaddr *) &cli_addr, (socklen_t *) &clilen); - EOF is not an error; errno is not set; don't use perror() on EOF 98 if (n==0) perror("EOF,Exit"); - don't put impossible tests in while loop - you can't be testing a value after using it (too late) - it's pointless to write an "else" after a program exit or return - repeatedly testing for end conditions inside a loop instead of breaking out and testing outside is wasteful and clutters your code - put rare code outside loop, not inside 117 while ((n = read(sock,buffer,sizeof(buffer)-1)) >= 0) 118 { 119 buffer[n] = '\0'; 120 if (n < 0) 121 { 122 error("ERROR reading from socket"); 123 exit (0); 124 } 129 else