Line/word wrap program.

[name]

Here is a first attempt at a line/word wrapping utility. Seems to work
okay, but lacks some checking stuff, etc.

---------------------------------------------------------

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>

#define MAX 10000

/* wrap.c inserts newlines in place of spaces according to specified
line length. Output filename is {filename}.wrap . Takes two arguments,
filename and line length. */

/* Todo: Need to figure out what sort of memory
the larger files might need. File type checking? */

void wordwrap(FILE *ifp, FILE *ofp, char *wl)
{
int c;

char buf[MAX];

int i, space, count, length;

i = space = count = 0;

length = atoi(wl);

for(i = 0; i < MAX && ((c=getc(ifp)) != EOF); ++i)
{
buf[i] = c;
}

for ( i = 0; buf[i] != '\0'; ++i)
{
if ((buf[i] == '\n' || buf[i] == '\t') && buf[i-1] == '\n')
count = space = 0;

if ( buf[i] == ' ')
space = i;

++count;

if ( count == length )
{
buf[space] = '\n';
count = i - space;
}
}

for ( i = 0; buf[i] != EOF; ++i)
{
c = buf[i];
putc(c, ofp);
}
}

int main(int argc, char *argv[])
{
FILE *fp1;
FILE *fp2;

char *prog = argv[0];
char *filename1 = argv[1];
char filename2[80];
char *wl = argv[2];
int i, l;

l = strlen(argv[1]);
for ( i = 0; i < 75 || i < l; ++i)
filename2[i] = argv[1][i];
strcat(filename 2, ".wrap");

printf("Wrappin g %s at %s\n", filename1, wl);
printf("Output file adds .wrap to input filename.\n");

if (argc != 3)
{
printf("Usage: %s: filename, wrap length\n", prog);
return EXIT_FAILURE;
}
else if ((fp1 = fopen(filename1 , "r")) == NULL)
{
fprintf(stderr, "%s: can't open %s\n", prog, filename1);
return EXIT_FAILURE;
}
else if ((fp2 = fopen(filename2 , "w")) == NULL)
{
fprintf(stderr, "%s: can't open %s\n", prog, filename2);
return EXIT_FAILURE;
}
else
{
wordwrap(fp1, fp2, wl);
fclose(fp1);
fclose(fp2);
}
if (ferror(fp2))
{
fprintf(stderr, "%s: error writing %s\n", prog, argv[3]);
return EXIT_FAILURE;
}

return EXIT_SUCCESS;
}

-----------------------------------------------------------

I presume it's ugly, so comments and criticisms welcome.

Thanks for reading.
--
Email is wtallman at olypen dot com

[Arthur J. O'Dwyer]

On Thu, 19 Aug 2004, name wrote:

Here is a first attempt at a line/word wrapping utility. Seems to work
okay, but lacks some checking stuff, etc.
When run with no command-line arguments, it segfaults.
When run with a filename and a numeric argument, it produces
way-incorrect results. Try each of the following invocations

% ./a.out test.txt
% ./a.out test.txt 10
% ./a.out test.txt 20
% ./a.out test.txt 21
% ./a.out test.txt 100

on a file 'test.txt' containing two lines of 20 'x's each.

xxxxxxxxxxxxxxx xxxxx
xxxxxxxxxxxxxxx xxxxx

I don't get the right answer in /any/ of those cases!

The code you've posted is indeed incredibly ugly. Remove some of
those blank lines, reduce your tabstop to something sensible (e.g., 4),
fix the obvious bug and any more you find along the way, and then
try posting again.

l = strlen(argv[1]);
This is the obvious bug.
for ( i = 0; i < 75 || i < l; ++i)
filename2[i] = argv[1][i];
strcat(filename 2, ".wrap");
This is one you should find along the way.
fprintf(stderr, "%s: error writing %s\n", prog, argv[3]);

So is this.
-Arthur,
http://www.contrib.andrew.cmu.edu/~a...e/usenetify2.c

[name]

On 2004-08-19, Arthur J. O'Dwyer <aj*@nospam.and rew.cmu.edu> wrote:

On Thu, 19 Aug 2004, name wrote:

Here is a first attempt at a line/word wrapping utility. Seems to work
okay, but lacks some checking stuff, etc.
When run with no command-line arguments, it segfaults.

Yep, I got that.
When run with a filename and a numeric argument, it produces
way-incorrect results. Try each of the following invocations

% ./a.out test.txt
That should give you the usage response. Did for me.
% ./a.out test.txt 10
% ./a.out test.txt 20
% ./a.out test.txt 21
Hmmmmmm..... Didn't encounter any spaces, just 'x' and '\n'. Okay, I think
this gives me a clue about checking for binary files as well. I'll think on
it.
% ./a.out test.txt 100
Got that and fixed it. Thanks.
on a file 'test.txt' containing two lines of 20 'x's each.

xxxxxxxxxxxxxxx xxxxx
xxxxxxxxxxxxxxx xxxxx

I don't get the right answer in /any/ of those cases!
You should have gotten the usage response for only one argument.
The code you've posted is indeed incredibly ugly. Remove some of
those blank lines, reduce your tabstop to something sensible (e.g., 4),
fix the obvious bug and any more you find along the way, and then
try posting again.
Not talking about style of presentation, but the code itself. Thanks for
your input, though.

l = strlen(argv[1]);

This is the obvious bug.

Okay....

for ( i = 0; i < 75 || i < l; ++i)
filename2[i] = argv[1][i];
strcat(filename 2, ".wrap");

This is one you should find along the way.

Don't see the problem here, except that this four line group should follow
the argc check. I suppose that with no argv[1], 'strlen' tries to access
something that doesn't exist, and that would cause a segfault. So the
argument does have to exist. If it is an emply array, however... have to
think on this. Thanks.

fprintf(stderr, "%s: error writing %s\n", prog, argv[3]);

So is this.

Yep, that should be 'filename2' instead of 'argv[3]'. Got it fixed. Thanks.

As I said, there needs to be a lot of different kinds of checking going on,
which is the next step, I guess.

Thanks for reading.
--
Email is wtallman at olypen dot com

[Arthur J. O'Dwyer]

On Fri, 20 Aug 2004, name wrote:

On 2004-08-19, Arthur J. O'Dwyer <aj*@nospam.and rew.cmu.edu> wrote:

On Thu, 19 Aug 2004, name wrote:

Here is a first attempt at a line/word wrapping utility. Seems to work
okay, but lacks some checking stuff, etc. [...]

% ./a.out test.txt
That should give you the usage response. Did for me.

Nope; it dereferences the null pointer 'wl' in 'main'.

% ./a.out test.txt 10
% ./a.out test.txt 20
% ./a.out test.txt 21

Hmmmmmm..... Didn't encounter any spaces, just 'x' and '\n'. Okay, I think
this gives me a clue about checking for binary files as well. I'll think on
it.

I have no idea what you're thinking of, but on my system each of these
dumps a bunch of zero bytes after the output data. One major problem
I noticed while idly looking for the bug:
for (i = 0; buf[i] != EOF; ++i) {
The loop condition may never evaluate to false, here. This is not
the bug that is causing the incorrect output, but it's a bug which
might cause an infinite loop on some platforms.

% ./a.out test.txt 100

Got that and fixed it. Thanks.

Gee, that was a /different/ bug? Crazy! Keep looking; I'm sure
you'll find plenty more bugs...

The code you've posted is indeed incredibly ugly. Remove some of
those blank lines, reduce your tabstop to something sensible (e.g., 4),
fix the obvious bug and any more you find along the way, and then
try posting again.

Not talking about style of presentation, but the code itself.

The code itself /is incredibly ugly/. The way you can fix this
problem is to remove the extra blank lines /from your code/. And
use a consistent rule to place whitespace /in your code/. Presentation
is very important in communication, and source code is nothing if not
communication. If I just wanted a program to /do/ something, I'd
use the binary executable. When I see source code, I expect it to
be readable. Google up D.E. Knuth's opinions on the subject; I pretty
much agree with him on this topic. :)

for ( i = 0; i < 75 || i < l; ++i)
filename2[i] = argv[1][i];
strcat(filename 2, ".wrap");

This is one you should find along the way.

Don't see the problem here [...]

Suppose 'strlen(argv[1]) > 75' and work from there. Pay particular
attention to the size of 'filename2' and the behavior of 'strcat'.

Thanks for reading.

You're welcome.

-Arthur,
three-day weekend

[Barry Schwarz]

On Thu, 19 Aug 2004 20:59:13 -0000, name <us**@host.doma in> wrote:

Here is a first attempt at a line/word wrapping utility. Seems to work
okay, but lacks some checking stuff, etc.

---------------------------------------------------------

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>

#define MAX 10000

/* wrap.c inserts newlines in place of spaces according to specified
line length. Output filename is {filename}.wrap . Takes two arguments,
filename and line length. */

/* Todo: Need to figure out what sort of memory
the larger files might need. File type checking? */

void wordwrap(FILE *ifp, FILE *ofp, char *wl)
{
int c;

char buf[MAX];

int i, space, count, length;

i = space = count = 0;

length = atoi(wl);

for(i = 0; i < MAX && ((c=getc(ifp)) != EOF); ++i)
{
buf[i] = c;
}

for ( i = 0; buf[i] != '\0'; ++i)
Here you stop the loop when buf[i] is '\0'.
{
if ((buf[i] == '\n' || buf[i] == '\t') && buf[i-1] == '\n')
If the first character is '\t', buf[i-1] does not exist. This will
invoke undefined behavior.
count = space = 0;

if ( buf[i] == ' ')
space = i;

++count;

if ( count == length )
{
buf[space] = '\n';
count = i - space;
}
}

for ( i = 0; buf[i] != EOF; ++i)
How did buf[i] ever get to be EOF? You do not store c in buf when c
is EOF.

Why does this loop stop at a different place than the previous one?
{
c = buf[i];
putc(c, ofp);
}
}

int main(int argc, char *argv[])
{
FILE *fp1;
FILE *fp2;

char *prog = argv[0];
char *filename1 = argv[1];
You never do anything with filename1 that you couldn't do with argv[1]
directly.
char filename2[80];
char *wl = argv[2];
Ditto.
int i, l;

l = strlen(argv[1]);
for ( i = 0; i < 75 || i < l; ++i)
If l (a really lousy choice for a one character name; for a while I
thought it was one, not ell) is greater than 75, you will not copy the
terminating '\0'. This will cause the following strcat to invoke
undefined behavior.

What happens when l > 80?
filename2[i] = argv[1][i];
Is there a reason you didn't use strncpy?
strcat(filename 2, ".wrap");

printf("Wrappin g %s at %s\n", filename1, wl);
printf("Output file adds .wrap to input filename.\n");

if (argc != 3)
This is too late. You have already played with argv[1] and [2]
without first checking to see if they exist.
{
printf("Usage: %s: filename, wrap length\n", prog);
return EXIT_FAILURE;
}
else if ((fp1 = fopen(filename1 , "r")) == NULL)
{
fprintf(stderr, "%s: can't open %s\n", prog, filename1);
return EXIT_FAILURE;
}
else if ((fp2 = fopen(filename2 , "w")) == NULL)
{
fprintf(stderr, "%s: can't open %s\n", prog, filename2);
return EXIT_FAILURE;
}
else
{
wordwrap(fp1, fp2, wl);
fclose(fp1);
fclose(fp2);
}
if (ferror(fp2))
{
fprintf(stderr, "%s: error writing %s\n", prog, argv[3]);
return EXIT_FAILURE;
}

return EXIT_SUCCESS;
}

-----------------------------------------------------------

I presume it's ugly, so comments and criticisms welcome.

Thanks for reading.

<<Remove the del for email>>

[name]

On 2004-08-20, Arthur J. O'Dwyer <aj*@nospam.and rew.cmu.edu> wrote:

On Fri, 20 Aug 2004, name wrote:

On 2004-08-19, Arthur J. O'Dwyer <aj*@nospam.and rew.cmu.edu> wrote:

On Thu, 19 Aug 2004, name wrote:

Here is a first attempt at a line/word wrapping utility. Seems to work
okay, but lacks some checking stuff, etc. [...] % ./a.out test.txt
That should give you the usage response. Did for me.

Nope; it dereferences the null pointer 'wl' in 'main'.

Well, looks like I've not got a clue here, then. No problem, this is all
about learning anyway... <grin>

% ./a.out test.txt 10
% ./a.out test.txt 20
% ./a.out test.txt 21

Hmmmmmm..... Didn't encounter any spaces, just 'x' and '\n'. Okay, I think
this gives me a clue about checking for binary files as well. I'll think on
it.

I have no idea what you're thinking of, but on my system each of these
dumps a bunch of zero bytes after the output data. One major problem
I noticed while idly looking for the bug:

for (i = 0; buf[i] != EOF; ++i) {

The loop condition may never evaluate to false, here. This is not
the bug that is causing the incorrect output, but it's a bug which
might cause an infinite loop on some platforms.

Ah yes, other platforms. Which means I'm not writing portable code then.
And that's a very big problem!

% ./a.out test.txt 100

Got that and fixed it. Thanks.

Gee, that was a /different/ bug? Crazy! Keep looking; I'm sure
you'll find plenty more bugs...

Oh probably... <grin>. That one was addressed by an out of range check.
Length had to be a positive integer less than or equal to 80. I think.

The code you've posted is indeed incredibly ugly. Remove some of
those blank lines, reduce your tabstop to something sensible (e.g., 4),
fix the obvious bug and any more you find along the way, and then
try posting again.
Not talking about style of presentation, but the code itself.

The code itself /is incredibly ugly/. The way you can fix this
problem is to remove the extra blank lines /from your code/. And
use a consistent rule to place whitespace /in your code/. Presentation
is very important in communication, and source code is nothing if not
communication. If I just wanted a program to /do/ something, I'd
use the binary executable. When I see source code, I expect it to
be readable. Google up D.E. Knuth's opinions on the subject; I pretty
much agree with him on this topic. :)

Oh okay. Sorry. I didn't understand that the way code is printed out with
spacing and so forth is considered as important as the code itself. I had
tried to make enough white space so that it was easy to read. I guess I
could get rid of all the white space and do K&R braces, but as I understand
it, K&R is a "hallowed format" that must always have 8 space indentations,
and you say that's not acceptable.

I'll read Knuth, of course. Thanks for the suggestion.

for ( i = 0; i < 75 || i < l; ++i)
filename2[i] = argv[1][i];
strcat(filename 2, ".wrap");

This is one you should find along the way.
Don't see the problem here [...]

Suppose 'strlen(argv[1]) > 75' and work from there. Pay particular
attention to the size of 'filename2' and the behavior of 'strcat'.

Well, I could give it 256, I suppose. I'll check this out and see what
happens. The idea with 75 was that the extension would make it 80
characters, but that was arbitrary in any case. More to it than that?
Guess I'll find out!

Thanks for reading.

You're welcome.

-Arthur,
three-day weekend

Thanks for the critique! I'll take it away and study on it.
--
Email is wtallman at olypen dot com

[name]

On 2004-08-20, Barry Schwarz <sc******@deloz .net> wrote:

On Thu, 19 Aug 2004 20:59:13 -0000, name <us**@host.doma in> wrote:
<snip>

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

Here you stop the loop when buf[i] is '\0'.

{
if ((buf[i] == '\n' || buf[i] == '\t') && buf[i-1] == '\n')

If the first character is '\t', buf[i-1] does not exist. This will
invoke undefined behavior.

Uh oh, incipient demonic rhinology! That means I need to check 'i' itself
to make sure it's not negative. Thanks!

count = space = 0;

if ( buf[i] == ' ')
space = i;

++count;

if ( count == length )
{
buf[space] = '\n';
count = i - space;
}
}

for ( i = 0; buf[i] != EOF; ++i)

How did buf[i] ever get to be EOF? You do not store c in buf when c
is EOF.

Ummm, right. Should also have been '\0'? But then I don't store c when
buf[i] is '\0' either. Maybe, having already set the buffer up, I really
don't need a conditional, so "for(i=0;i!=str len(buf[]);++i) or something
like that. Probably not, though... the conditional looks wrong, so
probably is.
Why does this loop stop at a different place than the previous one?
Just what I was going to ask! LOL!!!

{
c = buf[i];
putc(c, ofp);
}
}

int main(int argc, char *argv[])
{
FILE *fp1;
FILE *fp2;

char *prog = argv[0];
char *filename1 = argv[1];

You never do anything with filename1 that you couldn't do with argv[1]
directly.

True, but I decided it was easier to follow if I used the same assignment
name style for both file names.

char filename2[80];
char *wl = argv[2];

Ditto.

Yep.

int i, l;

l = strlen(argv[1]);
for ( i = 0; i < 75 || i < l; ++i)

If l (a really lousy choice for a one character name; for a while I
thought it was one, not ell) is greater than 75, you will not copy the
terminating '\0'. This will cause the following strcat to invoke
undefined behavior.

Yeah, I see what you mean. l for length, but the visual confusion makes it
inappropriate. I'll remember that. Thanks!
What happens when l > 80?

filename2[i] = argv[1][i];

Is there a reason you didn't use strncpy?

Hmmm.. in fact I thought about it when I was going to use strcat, but forgot
to look into that. Better choice, it would seem.

strcat(filename 2, ".wrap");

printf("Wrappin g %s at %s\n", filename1, wl);
printf("Output file adds .wrap to input filename.\n");

if (argc != 3)

This is too late. You have already played with argv[1] and [2]
without first checking to see if they exist.

Yep, I figured that out. Check the argc value first to see what was given
before playing with stuff that may not exist. If they don't exist...
segfault!! Got that from Arthur.

Hmmmm... lclint is my friend!!!! I think I'll have an extended visit there
before bringing more code here! Problem is, lclint is a bit laconic at
times...

Thanks for the critique, Barry!
--
Email is wtallman at olypen dot com

[Barry Schwarz]

On Fri, 20 Aug 2004 07:42:06 -0000, name <us**@host.doma in> wrote:

On 2004-08-20, Barry Schwarz <sc******@deloz .net> wrote:

On Thu, 19 Aug 2004 20:59:13 -0000, name <us**@host.doma in> wrote:

<snip>

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

Here you stop the loop when buf[i] is '\0'.

{
if ((buf[i] == '\n' || buf[i] == '\t') && buf[i-1] == '\n')

If the first character is '\t', buf[i-1] does not exist. This will
invoke undefined behavior.

Uh oh, incipient demonic rhinology! That means I need to check 'i' itself
to make sure it's not negative. Thanks!

count = space = 0;

if ( buf[i] == ' ')
space = i;

++count;

if ( count == length )
{
buf[space] = '\n';
count = i - space;
}
}

for ( i = 0; buf[i] != EOF; ++i)

How did buf[i] ever get to be EOF? You do not store c in buf when c
is EOF.

Ummm, right. Should also have been '\0'? But then I don't store c when
buf[i] is '\0' either. Maybe, having already set the buffer up, I really

Your first loop which calls getc (trimmed from your quote) never
checks for the character value '\0' so it will store that in buf.

snip
<<Remove the del for email>>

[name]

On 2004-08-21, Barry Schwarz <sc******@deloz .net> wrote:

<snip>

How did buf[i] ever get to be EOF? You do not store c in buf when c
is EOF.

Ummm, right. Should also have been '\0'? But then I don't store c when
buf[i] is '\0' either. Maybe, having already set the buffer up, I really

Your first loop which calls getc (trimmed from your quote) never
checks for the character value '\0' so it will store that in buf.

Yeah, I got that after looking at it. It does now. I'm going to repost the
results of addressing some of the points raised in this thread.

Thanks,
--
Email is wtallman at olypen dot com

[bayxarea-usenet]

Was this utility ever completed? I have a use for it actually.

Thanks,

John

name wrote:

Here is a first attempt at a line/word wrapping utility. Seems to work okay, but lacks some checking stuff, etc.

---------------------------------------------------------

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>

#define MAX 10000

/* wrap.c inserts newlines in place of spaces according to specified
line length. Output filename is {filename}.wrap . Takes two arguments, filename and line length. */

/* Todo: Need to figure out what sort of memory
the larger files might need. File type checking? */

void wordwrap(FILE *ifp, FILE *ofp, char *wl)
{
int c;

char buf[MAX];

int i, space, count, length;

i = space = count = 0;

length = atoi(wl);

for(i = 0; i < MAX && ((c=getc(ifp)) != EOF); ++i)
{
buf[i] = c;
}

for ( i = 0; buf[i] != '\0'; ++i)
{
if ((buf[i] == '\n' || buf[i] == '\t') && buf[i-1] == '\n')
count = space = 0;

if ( buf[i] == ' ')
space = i;

++count;

if ( count == length )
{
buf[space] = '\n';
count = i - space;
}
}

for ( i = 0; buf[i] != EOF; ++i)
{
c = buf[i];
putc(c, ofp);
}
}

int main(int argc, char *argv[])
{
FILE *fp1;
FILE *fp2;

char *prog = argv[0];
char *filename1 = argv[1];
char filename2[80];
char *wl = argv[2];
int i, l;

l = strlen(argv[1]);
for ( i = 0; i < 75 || i < l; ++i)
filename2[i] = argv[1][i];
strcat(filename 2, ".wrap");

printf("Wrappin g %s at %s\n", filename1, wl);
printf("Output file adds .wrap to input filename.\n");

if (argc != 3)
{
printf("Usage: %s: filename, wrap length\n", prog);
return EXIT_FAILURE;
}
else if ((fp1 = fopen(filename1 , "r")) == NULL)
{
fprintf(stderr, "%s: can't open %s\n", prog, filename1); return EXIT_FAILURE;
}
else if ((fp2 = fopen(filename2 , "w")) == NULL)
{
fprintf(stderr, "%s: can't open %s\n", prog, filename2); return EXIT_FAILURE;
}
else
{
wordwrap(fp1, fp2, wl);
fclose(fp1);
fclose(fp2);
}
if (ferror(fp2))
{
fprintf(stderr, "%s: error writing %s\n", prog, argv[3]); return EXIT_FAILURE;
}

return EXIT_SUCCESS;
}

-----------------------------------------------------------

I presume it's ugly, so comments and criticisms welcome.

Thanks for reading.
--
Email is wtallman at olypen dot com