C Language, Need comments on commenting program

Goran Milo
#1 / 12

Need comments on commenting program

Hello. I would apreciate if you could comment on this program. It is
suppose to read C source files, and print them together with some
comments. Indeed it does that, but I'd like to hear if there are any
faults in my program (I'm sure there are). I am also not sure if i
organized the program well so comments on that would be welcomed as
well. Thanks.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <ctype.h>

#define PROG argv[0] != NULL ? stripdir(argv[0]) : "comment.exe"

char *stripdir(char *file)
{
char *p = strrchr(file, '/');
if(!p) p = strrchr(file, '\\');
return p ? ++p : file;

Quote:

}

void comment_print(char *str)
{
struct comment
{
char *func;
char *desc;
};

struct comment c[] = {
{ "putchar", "Write a character to stdout (usualy a macro)." },
{ "fprintf", "Format and print data to stream." },
{ "sprintf", "Format and print data to string." },
{ "printf", "Format and print data to stdout." },
{ "scanf", "Scan and format input." },
};
unsigned long csize = sizeof c / sizeof *c;
unsigned long i;
char *p;
int j;

if((p = strchr(str, '\n')) != NULL)
*p = 0;

for(i = 0; i < csize; i++)
{
if((p = strstr(str, c[i].func)) != NULL)
{
for(j = 0; j < p - str; j++)
{
putchar(isspace(str[j]) ? str[j] : ' ');
}
printf("/* %s */\n", c[i].desc);
break;
}
}
puts(str);

Quote:

}

int main(int argc, char *argv[])
{
FILE *fp;
char buf[BUFSIZ];
int open = 0;
int i;

if(argc < 2)
{
fprintf(stderr, "%s: No input files\n", PROG);
return EXIT_FAILURE;
}

for(i = 1; i < argc; i++)
{
if((fp = fopen(argv[i], "r")) == NULL)
{
fprintf(stderr, "%s: Can't open \"%s\"\n", PROG, argv[i]);
fprintf(stderr, "%s\n", strerror(errno));
continue;
}
while(fgets(buf, BUFSIZ, fp) != NULL)
{
comment_print(buf);
}
fclose(fp);
open++;
}

return open ? EXIT_SUCCESS : EXIT_FAILURE;

Quote:

}

P.S.
If i redirect the stdout to a file (ie comment blah.c > redir.txt) and
if the blah.c does not exist the contents of the redir.txt file gets
deleted (it becomes blank). I thought fprintf(stderr, ...) is used to
avoid this, but that is obviusly not happening. Am I doing something
wrong or is this a normal behaviour under Win2000 console?

Sun, 29 Aug 2004 19:06:29 GMT

Tobias Oe
#2 / 12

Need comments on commenting program

Quote:

> Hello. I would apreciate if you could comment on this program. It is
> suppose to read C source files, and print them together with some
> comments. Indeed it does that, but I'd like to hear if there are any
> faults in my program (I'm sure there are). I am also not sure if i
> organized the program well so comments on that would be welcomed as
> well. Thanks.

> #include <stdio.h>
> #include <string.h>
> #include <stdlib.h>
> #include <errno.h>
> #include <ctype.h>

> #define PROG argv[0] != NULL ? stripdir(argv[0]) : "comment.exe"

To be on the safe side, you may want to add a some () here.
Also, this macro can only be called in a context where argv is in scope. In
your case this is main. So it would make sense to limit it's scope to main
as well.

Quote:

> char *stripdir(char *file)
> {
> char *p = strrchr(file, '/');
> if(!p) p = strrchr(file, '\\');
> return p ? ++p : file;
> }

if file is NULL, you have a problem

Quote:

> void comment_print(char *str)
> {
> struct comment
> {
> char *func;
> char *desc;
> };

> struct comment c[] = {
> { "putchar", "Write a character to stdout (usualy a macro)." },
> { "fprintf", "Format and print data to stream." },
> { "sprintf", "Format and print data to string." },
> { "printf", "Format and print data to stdout." },
> { "scanf", "Scan and format input." },
> };
> unsigned long csize = sizeof c / sizeof *c;

size_t is more apropriate than unsigned long (even if it's an alias for it
on your particular implementation). I don't like the name, maybe n_comments?

Quote:

> unsigned long i;
> char *p;
> int j;

> if((p = strchr(str, '\n')) != NULL)
> *p = 0;

> for(i = 0; i < csize; i++)
> {
> if((p = strstr(str, c[i].func)) != NULL)
> {
> for(j = 0; j < p - str; j++)
> {
> putchar(isspace(str[j]) ? str[j] : ' ');

I always put an extra pair of () arround the condition of ?: But that's a
matter of style

Quote:

> }
> printf("/* %s */\n", c[i].desc);
> break;
> }
> }
> puts(str);
> }

> int main(int argc, char *argv[])
> {

#define PROG ( (argv[0] != NULL) ? stripdir(argv[0]) : "comment.exe" )

Quote:

> FILE *fp;
> char buf[BUFSIZ];

I don't think it is smart to use BUFSIZ here. I would introduce a new macro
for this here. See later why.

Quote:

> int open = 0;
> int i;

> if(argc < 2)
> {
> fprintf(stderr, "%s: No input files\n", PROG);
> return EXIT_FAILURE;
> }

> for(i = 1; i < argc; i++)
> {
> if((fp = fopen(argv[i], "r")) == NULL)
> {
> fprintf(stderr, "%s: Can't open \"%s\"\n", PROG, argv[i]);
> fprintf(stderr, "%s\n", strerror(errno));

Only use strerrno if errno is !=0. Else you may end up getting error
messages like Error: Success, which doesn't make much sense. In fact, you
should save errno (for the calling functions), set it to zero, call a
function that may change it do something with it and reset it to the saved
value.

Quote:

> continue;

I'm not a fan of continue. I would use an else clause to the if, but again
that's a matter of style.

Quote:

> }
> while(fgets(buf, BUFSIZ, fp) != NULL)

Now suppose you want to test your code with a small buffer. You change your
declaration of char buf[BUFSIZ] to char buf[10]. You then need to change
the code here too. With a new macro for the size of the buffer this would
be automatic. An even nicer solution is to use sizeof buf here.

Quote:

> {
> comment_print(buf);
> }
> fclose(fp);
> open++;
> }

> return open ? EXIT_SUCCESS : EXIT_FAILURE;

#undef PROG

Quote:

> }

> P.S.
> If i redirect the stdout to a file (ie comment blah.c > redir.txt) and
> if the blah.c does not exist the contents of the redir.txt file gets
> deleted (it becomes blank). I thought fprintf(stderr, ...) is used to
> avoid this, but that is obviusly not happening. Am I doing something
> wrong or is this a normal behaviour under Win2000 console?

<SOT>
No the fprintf(stderr,...) prints to standard error (duh). This is a
different stream than stdout. That's why you don't get the error message
"Can't open blah.c" etc in redir.txt but on the screen (because your stderr
is attached to the screen).
The redir.txt is empty because the shell empties it for you before
executing comment. When there is no input file, comment doesn't write
anything to stdout so redir.txt remains empty.
If you're under unix (maybe this works with DOS too), try >> instead of >.
Then the shell doesn't delete what's in redir.txt and the output of comment
should be appended to what's already in there
<\SOT>
Tobias

Sun, 29 Aug 2004 23:11:02 GMT

David Rubi
#3 / 12

Need comments on commenting program

Quote:

> > #define PROG argv[0] != NULL ? stripdir(argv[0]) : "comment.exe"

> To be on the safe side, you may want to add a some () here.
> Also, this macro can only be called in a context where argv is in scope. In
> your case this is main. So it would make sense to limit it's scope to main
> as well.

A common practice I've seen is to declare a global variable

char *argv0;

and assign

argv0 = argv[0];

in main(). Then you can use argv0 anywhere to represent the program
name.

david

--
If 91 were prime, it would be a counterexample to your conjecture.
-- Bruce Wheeler

Mon, 30 Aug 2004 02:44:59 GMT

Bj?rn Augesta
#4 / 12

Need comments on commenting program

Quote:

> Hello. I would apreciate if you could comment on this program. It is
> suppose to read C source files, and print them together with some
> comments. Indeed it does that, but I'd like to hear if there are any
> faults in my program (I'm sure there are). I am also not sure if i
> organized the program well so comments on that would be welcomed as
> well. Thanks.

> #include <stdio.h>
> #include <string.h>
> #include <stdlib.h>
> #include <errno.h>
> #include <ctype.h>

> #define PROG argv[0] != NULL ? stripdir(argv[0]) : "comment.exe"

> char *stripdir(char *file)
> {
> char *p = strrchr(file, '/');
> if(!p) p = strrchr(file, '\\');
> return p ? ++p : file;
> }

> void comment_print(char *str)
> {
> struct comment
> {
> char *func;
> char *desc;
> };

> struct comment c[] = {
> { "putchar", "Write a character to stdout (usualy a macro)." },
> { "fprintf", "Format and print data to stream." },
> { "sprintf", "Format and print data to string." },
> { "printf", "Format and print data to stdout." },
> { "scanf", "Scan and format input." },
> };
> unsigned long csize = sizeof c / sizeof *c;
> unsigned long i;
> char *p;
> int j;

> if((p = strchr(str, '\n')) != NULL)
> *p = 0;

> for(i = 0; i < csize; i++)
> {
> if((p = strstr(str, c[i].func)) != NULL)

Hi there,
what happens if the program finds e.g. fscanf, sscanf, vsnprint, and all
the other functions which ends with the name of a function in your struct?

Bj?rn
[snip]

Mon, 30 Aug 2004 05:52:46 GMT

Steve Gravro
#5 / 12

Need comments on commenting program

Quote:

>> > #define PROG argv[0] != NULL ? stripdir(argv[0]) : "comment.exe"

>> To be on the safe side, you may want to add a some () here.
>> Also, this macro can only be called in a context where argv is in scope. In
>> your case this is main. So it would make sense to limit it's scope to main
>> as well.

>A common practice I've seen is to declare a global variable

> char *argv0;

>and assign

> argv0 = argv[0];

>in main(). Then you can use argv0 anywhere to represent the program
>name.

Better yet:

argv0 = argv[0] ? argv[0] : "default program name";

This will handle situations, rare as they may be, where argc == 0.

--
Never make a sweeping statement in comp.lang.c (except when...).
--Richard Heathfield in comp.lang.c

Mon, 30 Aug 2004 11:37:47 GMT

Daniel Fo
#6 / 12

Need comments on commenting program

Quote:

> >> > #define PROG argv[0] != NULL ? stripdir(argv[0]) : "comment.exe"

> >> To be on the safe side, you may want to add a some () here.
> >> Also, this macro can only be called in a context where argv is in scope.
In
> >> your case this is main. So it would make sense to limit it's scope to
main
> >> as well.

> >A common practice I've seen is to declare a global variable

> > char *argv0;

> >and assign

> > argv0 = argv[0];

> >in main(). Then you can use argv0 anywhere to represent the program
> >name.

> Better yet:

> argv0 = argv[0] ? argv[0] : "default program name";

Even better yet:

argv0 = ( argv[0] && *argv[0] ) ? argv[0] : "default program name";

-Daniel

Mon, 30 Aug 2004 11:49:05 GMT

Micah Cowa
#7 / 12

Need comments on commenting program

Quote:

> If i redirect the stdout to a file (ie comment blah.c > redir.txt) and
> if the blah.c does not exist the contents of the redir.txt file gets
> deleted (it becomes blank). I thought fprintf(stderr, ...) is used to
> avoid this, but that is obviusly not happening. Am I doing something
> wrong or is this a normal behaviour under Win2000 console?

fprintf() isn't what causes redir.txt to get clobbered. Typically,
redirection of this sort causes the file to be clobbered even before
the program is actually run. If you want to avoid this, you'll have
to avoid using redirection, and pass an optional output filename to
the program. Then you can open the file only when you know output is
ready to be printed to it.

HTH,
Micah

Tue, 31 Aug 2004 02:34:43 GMT

Goran Milo
#8 / 12

Need comments on commenting program

says...
[snip]

Quote:

> > #include <stdio.h>
> > #include <string.h>
> > #include <stdlib.h>
> > #include <errno.h>
> > #include <ctype.h>

> > #define PROG argv[0] != NULL ? stripdir(argv[0]) : "comment.exe"

> > char *stripdir(char *file)
> > {
> > char *p = strrchr(file, '/');
> > if(!p) p = strrchr(file, '\\');
> > return p ? ++p : file;
> > }

> > void comment_print(char *str)
> > {
> > struct comment
> > {
> > char *func;
> > char *desc;
> > };

> > struct comment c[] = {
> > { "putchar", "Write a character to stdout (usualy a macro)." },
> > { "fprintf", "Format and print data to stream." },
> > { "sprintf", "Format and print data to string." },
> > { "printf", "Format and print data to stdout." },
> > { "scanf", "Scan and format input." },
> > };
> > unsigned long csize = sizeof c / sizeof *c;
> > unsigned long i;
> > char *p;
> > int j;

> > if((p = strchr(str, '\n')) != NULL)
> > *p = 0;

> > for(i = 0; i < csize; i++)
> > {
> > if((p = strstr(str, c[i].func)) != NULL)

> Hi there,
> what happens if the program finds e.g. fscanf, sscanf, vsnprint, and all
> the other functions which ends with the name of a function in your struct?

Is this a trick question? It prints their comments right above them.

Wed, 01 Sep 2004 01:12:04 GMT

Goran Milo
#9 / 12

Need comments on commenting program

Quote:

[snip]
> > >in main(). Then you can use argv0 anywhere to represent the program
> > >name.

> > Better yet:

> > argv0 = argv[0] ? argv[0] : "default program name";

> Even better yet:

> argv0 = ( argv[0] && *argv[0] ) ? argv[0] : "default program name";

Not sure I get the point? Why can't we just check argv[0]?

Wed, 01 Sep 2004 01:12:25 GMT

Goran Milo
#10 / 12

Need comments on commenting program

Quote:

> > #define PROG argv[0] != NULL ? stripdir(argv[0]) : "comment.exe"

> To be on the safe side, you may want to add a some () here.
> Also, this macro can only be called in a context where argv is in scope. In
> your case this is main. So it would make sense to limit it's scope to main
> as well.

Instead of that I did what Rubin suggested, cool trick.

Quote:

> > char *stripdir(char *file)
> > {
> > char *p = strrchr(file, '/');
> > if(!p) p = strrchr(file, '\\');
> > return p ? ++p : file;
> > }

> if file is NULL, you have a problem

Should I work around it? Most str* functions have problems if you pass
them a NULL pointer.

Quote:

> > void comment_print(char *str)
> > {
> > struct comment
> > {
> > char *func;
> > char *desc;
> > };

> > struct comment c[] = {
> > { "putchar", "Write a character to stdout (usualy a macro)." },
> > { "fprintf", "Format and print data to stream." },
> > { "sprintf", "Format and print data to string." },
> > { "printf", "Format and print data to stdout." },
> > { "scanf", "Scan and format input." },
> > };
> > unsigned long csize = sizeof c / sizeof *c;

> size_t is more apropriate than unsigned long (even if it's an alias for it
> on your particular implementation).

Ok I'll change that.

Quote:

> > FILE *fp;
> > char buf[BUFSIZ];

> I don't think it is smart to use BUFSIZ here. I would introduce a new macro
> for this here. See later why.

I've changed the BUFSIZE to sizeof buf down in the fgets() function.
That seems to be the best solution, without adding another macro, but
also without having to replace BUFISIZ with another value in more than
one place.

Wed, 01 Sep 2004 01:18:06 GMT

Goran Milo
#11 / 12

Need comments on commenting program

[snip]

Quote:

> > Hi there,
> > what happens if the program finds e.g. fscanf, sscanf, vsnprint, and all
> > the other functions which ends with the name of a function in your struct?

> Is this a trick question? It prints their comments right above them.

Oh I get your point now. Thats not too hard to work around, I'll just
check if the function's name is at the begining of a string, if not I'll
check with isalpha() whether there are any letters right before it.

Wed, 01 Sep 2004 01:27:08 GMT

Daniel Fo
#12 / 12

Need comments on commenting program

Quote:

> > Even better yet:

> > argv0 = ( argv[0] && *argv[0] ) ? argv[0] : "default program name";

> Not sure I get the point? Why can't we just check argv[0]?

Even if argv[0] is not null, it may be an empty string if the program name is
not available from the environment.

-Daniel

Wed, 01 Sep 2004 05:44:31 GMT