First C program

[Mike Wahler]

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 !!!

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.
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.
main() */

Correct. Though preferred would be

int main(void)


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) */


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>
}

}
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.
octet3);

You need to check that 'fprintf()' succeeded here.
See your compiler manual or textbook to find out how.

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

 

[Allin Cottrell]

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 !!!

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 "translation" 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_FAILURE);
}

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;
}

 

[David Rubin]

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 !!!

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

 

[Mike Wahler]

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

 

[Mac]

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 !!!

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
--

 

[Noah Roberts]

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]

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

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.

 

[Nick Austin]

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]

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

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

Irrwahn

 

[Robert Stankowic]

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(...) *?

Hey, thanks for the idea
Very handy in code, where globals cannot be avoided (<OT>A DLL for example,
with a bunch of functions isolated from each other, but using the same
variables, and no, I don't like the idea to manage them in the calling VB
application </OT>)

Robert

 

[hokieghal99]

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.

Thanks, my mailer Mozilla Mail would not let me paste the source into
the body of the email unless I pasted it as a quotation. I'm soryy...
I'll remove all the '>' in the future. BTY, I wrote this myself from a
Python script that I wrote. I thought it would be a good way to start
out... converting Python scripts to C programs.

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?

Practical C Programming, 3rd Edition
by Steve Oualline

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)

The author of my book wants everything commented. I guess it's a matter
of style, but I can see your point. Good names go a long way toward
being a comment. Thanks for the tip.

main() */

Correct. Though preferred would be

int main(void)

I do not understand this. Why is this preferred over int main()?

Thanks for all the tips!!!

 

[hokieghal99]

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

Thank you for the example. My programming background is limited to
systems administration (Python, Perl, Bash) so my idea of programming is
limited and probably somewhat pragmatic. C will be a learning experience
for me. Thanks again!!!

#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_FAILURE);
}

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;
}

 

[Nils Petter Vaskinn]

The author of my book wants everything commented. I guess it's a matter
of style, but I can see your point. Good names go a long way toward
being a comment. Thanks for the tip.

Comments in code in a programming textbook are meant to be read by people
that doesn't know the language (yet). Comments in ordinary code is
supposed to be read by people that know the language (and so won't need an
explanation for most of the code). So instead of commenting everything you
should only comment the important bits, so that they are not lost among
all the other comments.


regards

 

[Noah Roberts]

Comments in code in a programming textbook are meant to be read by people
that doesn't know the language (yet). Comments in ordinary code is
supposed to be read by people that know the language (and so won't need an
explanation for most of the code). So instead of commenting everything you
should only comment the important bits, so that they are not lost among
all the other comments.

Usually you can comment a short description of what the stuff in this
file is for, a comment header over functions to describe input and
expected output, and comment areas of code that are particularly
obfuscated. You may also wish to comment use of globals for instance
something like:

global_x = new_value; /* global_x is defined in globals.c */

You don't need much more than these things and sometimes even they are
unnecissary. What you don't want is code without comments, so find a
balance that maintains code readibility but not more.

NR

 

[hokieghal99]

Something I just discovered. I can declare a variable and initialize it
at the same time... this is a big time/line saver. Is this inappropiate?

int octet0 = 192;
int octet1 = 168;
int octet2 = 1;
int octet3 = 1;

 

[Mike Wahler]

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

I made a comment about that in my original reply.
I left 'how to do it' for him to research on his own.

-Mike

 

[Mike Wahler]

Something I just discovered.

I showed you that in my first reply.

I can declare a variable and initialize it
at the same time... this is a big time/line saver. Is this inappropiate?

int octet0 = 192;
int octet1 = 168;
int octet2 = 1;
int octet3 = 1;

It's *very* appropriate, for a few reasons:

1. (as you say above), it keeps the code more concise
(and still quite clear)

2. Readers of the code don't have to search around to
find where the objects first get their values.

3. (imo most important), if you forget to give them
values at all, and try to evalutate their values,
you get the dreaded 'undefined behavior.' I wrote
about this in my first reply.

-Mike

 

[Mike Wahler]

Thanks, my mailer Mozilla Mail would not let me paste the source into
the body of the email unless I pasted it as a quotation. I'm soryy...
I'll remove all the '>' in the future. BTY, I wrote this myself from a
Python script that I wrote. I thought it would be a good way to start
out... converting Python scripts to C programs.

I don't think that's a good way at all. But that's
just my opinion. You run the risk of trying to apply
semantics, 'patterns' and idioms that make sense with
the old language where they might not make sense with
the new one. I see this all the time with folks moving
from/to C and C++. Moving from C to C++, I stumbled a
bit with this myself. The folks over at clc++ were
a great aid with this.

Practical C Programming, 3rd Edition
by Steve Oualline




The author of my book wants everything commented. I guess it's a matter
of style, but I can see your point. Good names go a long way toward
being a comment. Thanks for the tip.



I do not understand this. Why is this preferred over int main()?

Well, it's not a real critical issue, but let me explain:

An empty function argument list in C does not mean
'no arguments', it means 'number and type(s) of argument(s)
is unspecified.' An argument list of (void) specifically
means 'no arguments'. The only valid argument lists for
main() are none at all, or
(int, char**, /* more optional implementation-defined ones */)

The () form is essentially obsolete (but kept for back-compatibility)

-Mike

 

[Ed Morton]

On 9/30/2003 10:26 AM, Noah Roberts wrote:

obfuscated. You may also wish to comment use of globals for instance
something like:

global_x = new_value; /* global_x is defined in globals.c */

If you move the definition of global_x from globals.c to bubba.c, you wouldn't
want to have to go and find everywhere you used global_x and change the
comments, so don't ever add a comment that needlessly couples your code like this.

Ed.

 

[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 !!!