First C program

[hokiegal99]

Hi Guys,

This is my first C program. I started programming in Python. Please look
over this source and let me know if I'm doing anything wrong. I want to
start out right with C, so if you see anything wrong tell me now while I
am learning!

Thanks !!!

#include <stdio.h>

int octet0; /* Declare first octet of our IP range */
int octet1; /* Declare second octet of our IP range */
int octet2; /* Declare third octet of our IP range */
int octet3; /* Declare fourth octet (the one we generate) of our IP range */

FILE *outputFile; /* Declare the file in which we will write the IP range to */

int main() /* This is the correct way to declare main... do not use void main() */
{

octet0 = 192; /* Value of first octet */
octet1 = 168; /* Value of second octet */
octet2 = 1; /* Value of third octet */

outputFile = fopen("ips_c.tx t", "wt"); /* Create a text file, open it in write mode */
for (octet3 = 1; octet3 < 255; ++octet3) {
/* printf ("%d.%d.%d.%d\n ", octet0, octet1, octet2, octet3); */
fprintf (outputFile, "%d.%d.%d.%d\n" , octet0, octet1, octet2, octet3);
}
fclose(outputFi le);
printf ("\nThe file 'ips_c.txt' was generated successfully... \n\n");
return(0);
}

[Mike Wahler]

"hokiegal99 " <ho********@hot mail.com> wrote in message
news:3F******** ******@hotmail. com...

Hi Guys,

This is my first C program. I started programming in Python. Please look
over this source and let me know if I'm doing anything wrong. I want to
start out right with C, so if you see anything wrong tell me now while I
am learning!
Thou hast already committed thy first sin, my son. :-)

The first C program one should write is the classic
"Hello world".

But let's take a look.

First, if you share code here, if at all possible post
*compilable* code. Of course the fastest way for anyone
to spot any syntax errors is to paste your code into a
compiler and compile it. This I did, but first I had
to spend time deleting all those > characters. Actually,
their presence makes me suspect you copied that out of
a newsgroup post or email message.

Also, learn to indent your code. What does the example
code in your textbook look like? Is it all left aligned
like that? Which textbook(s) are you reading?


Thanks !!!

#include <stdio.h>

int octet0; /* Declare first octet of our IP range */
int octet1; /* Declare second octet of our IP range */
int octet2; /* Declare third octet of our IP range */
int octet3; /* Declare fourth octet (the one we generate) of our IP range */

1. Those comments are worse than useless. They only clutter
the code and give no additional information. The identifier
names you're using (which are pretty good ones btw) already
convey the information in the comments (except the 'IP' part)

If I commented this part at all, I'd write something like:

/* IP address octets */
int octet0;
int octet1;
int octet2;
int octet3;

Actually I'd put these values in an array, but you're probably
not that far with the language yet.

2. There's no reason to put these at file scope. They should
go inside your 'main()' function, where they're used.

FILE *outputFile; /* Declare the file in which we will write the IP range to */

Another useless comment. Your (again good) identifier name
'outputFile' tells the story : it's an output file. If you
want to convey that the file will contain 'IP addresses',
perhaps a name like 'IPFileOut' or similar might be closer.

This object should also be defined inside your 'main()'
function, not here.

int main() /* This is the correct way to declare main... do not use void main() */

Correct. Though preferred would be

int main(void)

{

octet0 = 192; /* Value of first octet */
octet1 = 168; /* Value of second octet */
octet2 = 1; /* Value of third octet */
More useless comments.

Also, these values could be specified at definition time
with initializers (these definitions should be inside
this function anyway):

int octet0 = 192;
int octet1 = 168;
int octet2 = 1;
int octet3 = 0;
/* technically initialization of 'octet3' is not necessary,
since you assign it a value below before using it. But
I find that *always* initializing *every* object with
*some* valid value is a good defensive practice. If
you don't give an initial value, and forget to do so
before trying to access it, you get 'undefined behavior.'
Not Good(tm) */

outputFile = fopen("ips_c.tx t", "wt"); /* Create a text file, open it in write mode */

There's no standard 'open mode' of "wt". C streams are
'text mode' by default, unless otherwise specified.
Just use "w".

Also, very important is that you check that the call to
'fopen()' succeeded before trying to touch the file.
If 'fopen()' failed, any attempts at i/o result in,
you guessed it, undefined behavior. :-)

if(outputFile == NULL)
{
puts("Cannot open input");
return EXIT_FAILURE; /* 'EXIT_FAILURE' needs #include <stdlib.h>
}

} for (octet3 = 1; octet3 < 255; ++octet3) {
This will iterate for values of 'octet3' of 1 through
254 inclusive. If you want to include 255, change the
< to <= or compare against 256 with <. I also see you
start with 1 and not zero. This range of values may be
what you want, I don't know. Just drawing attention to
it just in case.
/* printf ("%d.%d.%d.%d\n ", octet0, octet1, octet2, octet3); */
fprintf (outputFile, "%d.%d.%d.%d\n" , octet0, octet1, octet2, octet3);

You need to check that 'fprintf()' succeeded here.
See your compiler manual or textbook to find out how.
}
fclose(outputFi le);
printf ("\nThe file 'ips_c.txt' was generated successfully... \n\n");
return(0);
Nitpick: This is valid, but the parentheses are not
needed. 'return' is not a function, it's a keyword.
}

All in all, not bad. But again, you first should have
written the "Hello world" program. Now go forth and
sin no more. :-)

-Mike

[Mac]

On Tue, 30 Sep 2003 16:25:14 +0000, hokiegal99 wrote:

Hi Guys,

This is my first C program. I started programming in Python. Please look
over this source and let me know if I'm doing anything wrong. I want to
start out right with C, so if you see anything wrong tell me now while I
am learning!

Thanks !!!

#include <stdio.h>

int octet0; /* Declare first octet of our IP range */
int octet1; /* Declare second octet of our IP range */
int octet2; /* Declare third octet of our IP range */
int octet3; /* Declare fourth octet (the one we generate) of our IP range */

FILE *outputFile; /* Declare the file in which we will write the IP range to */

int main() /* This is the correct way to declare main... do not use void main() */
{

octet0 = 192; /* Value of first octet */
octet1 = 168; /* Value of second octet */
octet2 = 1; /* Value of third octet */

outputFile = fopen("ips_c.tx t", "wt"); /* Create a text file, open it in write mode */
for (octet3 = 1; octet3 < 255; ++octet3) {
/* printf ("%d.%d.%d.%d\n ", octet0, octet1, octet2, octet3); */
fprintf (outputFile, "%d.%d.%d.%d\n" , octet0, octet1, octet2, octet3);
}
fclose(outputFi le);
printf ("\nThe file 'ips_c.txt' was generated successfully... \n\n");
return(0);
}
>

Mike Wahler already critiqued the code. I'll just add that fclose()
returns a value, so if you are going to claim that you successfully wrote
the file, you should probably check the return value of fclose().

Mac
--

[Allin Cottrell]

hokiegal99 wrote:

Hi Guys,

This is my first C program. I started programming in Python. Please look
over this source and let me know if I'm doing anything wrong. I want to
start out right with C, so if you see anything wrong tell me now while I
am learning!

Thanks !!!

#include <stdio.h>

int octet0; /* Declare first octet of our IP range */
int octet1; /* Declare second octet of our IP range */
int octet2; /* Declare third octet of our IP range */
int octet3; /* Declare fourth octet (the one we generate) of
our IP range */

FILE *outputFile; /* Declare the file in which we will write the IP
range to */

int main() /* This is the correct way to declare main... do not
use void main() */
{

octet0 = 192; /* Value of first octet */
octet1 = 168; /* Value of second octet */
octet2 = 1; /* Value of third octet */

outputFile = fopen("ips_c.tx t", "wt"); /* Create a text file,
open it in write mode */
for (octet3 = 1; octet3 < 255; ++octet3) {
/* printf ("%d.%d.%d.%d\n ", octet0, octet1, octet2, octet3); */
fprintf (outputFile, "%d.%d.%d.%d\n" , octet0, octet1, octet2,
octet3);
}
fclose(outputFi le);
printf ("\nThe file 'ips_c.txt' was generated successfully... \n\n");
return(0);
}

Nice work for a C newbie. The only "error" I can see is not checking
that fopen has worked before writing to your file -- oh, and giving
a mode of "wt" rather than "w" to fopen(). And maybe getting the range
of your "for" loop off-by-one.

As you might expect, though, this is not very idiomatic C. Here's
a "translatio n" of your code that sticks close to the orginal in
spirit, but that is more idiomatic:

#include <stdio.h>
#include <stdlib.h> /* for exit statuses */

int main (void)
{
const char *outfile = "ips_c.txt" ;
FILE *fp; /* it's not usually necessary to give very descriptive
names to streams -- it's generally pretty clear from
context what they're for.
*/
int ip_octet[4];
int i;

ip_octet[0] = 192;
ip_octet[1] = 168;
ip_octet[2] = 1;

fp = fopen(outfile, "w");
if (fp == NULL) {
fprintf(stderr, "Failed to open '%s' for writing\n", outfile);
exit(EXIT_FAILU RE);
}

for (i=0; i<255; i++) {
ip_octet[3] = i + 1;
fprintf(fp, "%d.%d.%d.%d\n" , ip_octet[0], ip_octet[1],
ip_octet[2], ip_octet[3]);
}

fclose(fp);

printf("The file '%s' was was generated successfully\n" ,
outfile);

return 0;
}
--
Allin Cottrell
Department of Economics
Wake Forest University, NC

[David Rubin]

hokiegal99 wrote:

Hi Guys,

This is my first C program. I started programming in Python. Please look
over this source and let me know if I'm doing anything wrong. I want to
start out right with C, so if you see anything wrong tell me now while I
am learning!

Thanks !!!

#include <stdio.h>

int octet0; /* Declare first octet of our IP range */
int octet1; /* Declare second octet of our IP range */
int octet2; /* Declare third octet of our IP range */
int octet3; /* Declare fourth octet (the one we generate) of
our IP range */

FILE *outputFile; /* Declare the file in which we will write the IP
range to */

int main() /* This is the correct way to declare main... do not
use void main() */
{

octet0 = 192; /* Value of first octet */
octet1 = 168; /* Value of second octet */
octet2 = 1; /* Value of third octet */

outputFile = fopen("ips_c.tx t", "wt"); /* Create a text file,
open it in write mode */
for (octet3 = 1; octet3 < 255; ++octet3) {
/* printf ("%d.%d.%d.%d\n ", octet0, octet1, octet2, octet3); */
fprintf (outputFile, "%d.%d.%d.%d\n" , octet0, octet1, octet2,
octet3);
}
fclose(outputFi le);
printf ("\nThe file 'ips_c.txt' was generated successfully... \n\n");
return(0);
}

The objective of your program is to print the string representation of dotted
decimal IP addresses in a particular network. You have the right idea, but you
are being a bit overzealous. How about this...

#include <stdio.h>

int
main(void)
{
int host;

for(host=1; host < 256; ++host)
printf("192.168 .1.%d\n", host);
return 0;
}

Invoke as

./a.out > ips_c.txt

or embed the file I/O in the program as before.

/david

--
FORTRAN was the language of choice
for the same reason that three-legged races are popular.
-- Ken Thompson, "Reflection s on Trusting Trust"

[Mike Wahler]

"Mac" <fo*@bar.net> wrote in message
news:pa******** *************** *****@bar.net.. .

Mike Wahler already critiqued the code. I'll just add that fclose()
returns a value, so if you are going to claim that you successfully wrote
the file, you should probably check the return value of fclose().

I always forget something. :-) Thanks.

-Mike

[Noah Roberts]

Mike Wahler wrote:

"hokiegal99 " <ho********@hot mail.com> wrote in message
news:3F******** ******@hotmail. com...

#include <stdio.h>

int octet0; /* Declare first octet of our IP range */
int octet1; /* Declare second octet of our IP range */
int octet2; /* Declare third octet of our IP range */
int octet3; /* Declare fourth octet (the one we generate) of our IP

range */

Something I would add on this level is that using an unsigned char
instead of int might be prudent in many cases like the above. You have
already declaired that they are "octets" which by definition are single
bytes (on any modern day computer) so you could just use bytes. Don't
use simply "char" because it is compiler specific whether or not "char"
is signed, so make sure by calling it unsigned explicitly.

You could also get fancy and pad a known 4 byte integer; in reality this
is what an IP address is. At the level of this program though that is
undue complexity and overhead.

NR

[Charles Harrison Caudill]

hokiegal99 <ho********@hot mail.com> wrote:

This is my first C program. I started programming in Python. Please look

<snip>

int octet0; /* Declare first octet of our IP range */
int octet1; /* Declare second octet of our IP range */
int octet2; /* Declare third octet of our IP range */
int octet3; /* Declare fourth octet (the one we generate) of our IP range */

toss the globals. They make for confusing code in the end. Learn to use
structs. a convenient way to do it is:

typedef struct _foo_t {
int bar;
char *baz;
struct _foo *recursive;
} foo_t;

The _t at the end quickly identifies it as a datatype, and emacs will
highlight it as a datatype for you; I'm an emacs user so that makes things
nice for me. If you *must* have globals for things like handling signals
then you can always access them like so:

int access_foo(int new, int do_set){
static int foo = 0;
if(do_set){
foo = new;
return 0;
} /* end of if */
else {
return foo;
} /* end of else */
} /* end int access_foo(...) *?

Effectively you are using globals, but the wrapper allows you to later on
do whatever you want, perhaps look it up in a database or something.
As someone who's had to debug 22,000 lines of spaghetti code w/ globals-
galore, written by 4 different physicists over 15 years, I know the dangers
of using globals. Don't make the same mistake.

Also, my personal style is to create a foo.h for every foo.c

I put everything that the rest of the world would ever want to know about
foo.c in foo.h, which does *not* include many #defines, or *any*
static function declarations; o yeah, any function that does not need to
be used by another .c file should be declared static so as not to clutter
the namespace. If you look around in large c programs you see enormous
function names because ansi c does not support namespaces.

Most c coders also use names separated by _'s instead of mixing the
capitalization, at about 6 in the morning when you're brain is incapable
of inserting the space you'll begin to appreciate that tactic.

kudos to you for playing w/ c by the way.

--
Harrison Caudill | .^ www.hypersphere.org
Computer Science & Physics Double Major | | Me*Me=1
Georgia Institute of Technology | '/ I'm just a normal guy

[Nick Austin]

On Mon, 29 Sep 2003 20:07:41 -0700, "Mac" <fo*@bar.net> wrote:

On Tue, 30 Sep 2003 16:25:14 +0000, hokiegal99 wrote:

Hi Guys,

This is my first C program. I started programming in Python. Please look
over this source and let me know if I'm doing anything wrong. I want to
start out right with C, so if you see anything wrong tell me now while I
am learning!

Thanks !!!

#include <stdio.h>

int octet0; /* Declare first octet of our IP range */
int octet1; /* Declare second octet of our IP range */
int octet2; /* Declare third octet of our IP range */
int octet3; /* Declare fourth octet (the one we generate) of our IP range */

FILE *outputFile; /* Declare the file in which we will write the IP range to */

int main() /* This is the correct way to declare main... do not use void main() */
{

octet0 = 192; /* Value of first octet */
octet1 = 168; /* Value of second octet */
octet2 = 1; /* Value of third octet */

outputFile = fopen("ips_c.tx t", "wt"); /* Create a text file, open it in write mode */
for (octet3 = 1; octet3 < 255; ++octet3) {
/* printf ("%d.%d.%d.%d\n ", octet0, octet1, octet2, octet3); */
fprintf (outputFile, "%d.%d.%d.%d\n" , octet0, octet1, octet2, octet3);
}
fclose(outputFi le);
printf ("\nThe file 'ips_c.txt' was generated successfully... \n\n");
return(0);
}
>

Mike Wahler already critiqued the code. I'll just add that fclose()
returns a value, so if you are going to claim that you successfully wrote
the file, you should probably check the return value of fclose().

For that matter, there could also be an error during fprintf().

Nick.

[Irrwahn Grausewitz]

Nick Austin <ni**********@n ildram.co.uk> wrote:

On Mon, 29 Sep 2003 20:07:41 -0700, "Mac" <fo*@bar.net> wrote:

On Tue, 30 Sep 2003 16:25:14 +0000, hokiegal99 wrote: <SNIP>

outputFile = fopen("ips_c.tx t", "wt"); /* Create a text file, open it in write mode */
for (octet3 = 1; octet3 < 255; ++octet3) {
/* printf ("%d.%d.%d.%d\n ", octet0, octet1, octet2, octet3); */
fprintf (outputFile, "%d.%d.%d.%d\n" , octet0, octet1, octet2, octet3);
}
fclose(outputFi le);
printf ("\nThe file 'ips_c.txt' was generated successfully... \n\n");
return(0);
}
>

Mike Wahler already critiqued the code. I'll just add that fclose()
returns a value, so if you are going to claim that you successfully wrote
the file, you should probably check the return value of fclose().

For that matter, there could also be an error during fprintf().

.... as Mike already pointed out in his first reply.

Irrwahn
--
Great minds run in great circles.