Re: K&R exercise 1-18
- From: Michael Mair <Michael.Mair@xxxxxxxxxxxxxxx>
- Date: Wed, 21 Jun 2006 02:41:31 +0200
Eric schrieb:
I have been working through the K&R book (only chapter one so far) examples
and exercises. After much sweat and hair pulling, I think I have a
solution for ex 1-18 on page 31.
Not everyone has the exercise texts handy; just copy them (if not
too long).
It seems to work but may be missing some error checking. Can you please
take a look and see if my logic is correct and any other improvements.
Yes, of course.
/* function to strip trailing blanks and tabs from the input line */
/* and not to print any blank lines */
#include <stdio.h>
#define MAXIMUM 1000 /* maximum line length including '\0' marker */
Note: Often, one declares MAXIMUM to be the number of characters
_without_ the string terminator and has MAXIMUM+1 array elements.
This has the advantage that you have not to subtract 1 which happens
too often otherwise.
void remblank(char line[]);
Your prototype's parameter name and the parameter name
in the function definition below do not match. Try to keep
them in sync -- some compilers warn you about the mismatch
as it can indicate a semantic change not communicated to
all parties.
char line[] is in this context equivalent to char *line
but obscures the fact that you are really dealing with
a pointer.
void remblank (char *s);
Actually, I will suggest a signature change further down
the road...
int main()
int main (void)
is the preferred form around here -- it leaves absolutely
no doubt.
{
int c, i;
Note: c and i are used for different purposes; the former stores
characters cast to unsigned char or EOF (the return value of
getchar()), the latter is used as index.
Even if they have the same type, I'd rather separate them.
int c;
int i;
i = 0;
char line[MAXIMUM];
Mixing statements and declarations makes your code "C99 only"
without necessity where it could be valid C90 and valid C99.
char line[MAXIMUM];
i = 0;
c = getchar();
while (c != EOF)
{
line[i] = c;
if (i == (MAXIMUM - 1)) /* make sure line is <= 1000 */
Here is one of the mentioned MAXIMUM-1 instances.
line[i] = '\n';
Note: You lose one character by overwriting it with '\n'.
Try
#define MAXIMUM 8
and give
123456789012345678901234567890
as input. ungetc() or storing the excess character can solve
this problem. For yet another solution see below.
if (line[i] == '\n')
{
line[i + 1] = '\0';
If i==(MAXIMUM-1), then i+1==MAXIMUM, i.e. you access a position
one _past_ the end of the array.
remblank(line);
i = 0;
}
else
++i;
c = getchar();
}
/* print the last bit if there is more */
/* after the last newline */ if (c == EOF)
{
Note: The previous loop only is terminated on c == EOF,
so the test is unnecessary.
You probably meant to test
if (i != 0)
line[i] = '\0';
while (line[i - 1] == ('\t') || line[i - 1] == (' '))
If i==0, then you are accessing something one outside the array.
{
line[i - 1] = '\0';
--i;
}
Now you are re-building the functionality of remblank() but
for '\n'.
printf("%s", line);
Note: The last output character should be '\n' -- otherwise you
might not see the last "line".
In the light of this:
Why don't you omit the '\n' from your line? Then you can
use remblank() whenever you need it; remblank() can output
via puts() which automatically adds a '\n'.
This can solve your above problem partially.
}
return 0;
}
void remblank(char s[])
{
int i;
for(i = 0; s[i] != '\n'; i++) /* count characters in line */
;
You are calculating a number you already had.
If you pass not only s but also the "length" of s, you safe
the second time.
/* if character before newline is a blank or tab */
/* replace it with a newline and make the next */
/* character an end of array marker */
while (s[i - 1] == ('\t') || s[i - 1] == (' '))
For a blank line (i==0), you are accessing storage outside
your array. This leads to undefined behaviour.
As you are even writing to this storage, you can for example
inadvertently change other variables.
If you are very unlucky, much of the storage before s contains
a ' ' or '\t' representation -- the programme runs amok through
your memory.
while (i != 0 && (....))
is safer. Or you can switch to a for loop counting down i
from which you break if s[i]!='\t'.
{
s[i -1] = '\n';
s[i] = '\0';
--i;
}
/* if line is not blank, print the line without */
/* the trailing blanks or tabs */
if(s[0] != '\n')
I'd rather check i != 0.
printf("%s", s);
}
Additional comments:
- In main(), you are reading characters line by line but your
loop structure does not reflect that; this can make life difficult
if you want to add functionality.
- You are explicitly testing for '\t' and ' '. Consider wrapping
this into a function of its own, e.g.
int isRemovable (int c)
{
static const char * const removableChars = "\t ";
/* This is not exactly equivalent to your test, as
** it also returns 1 for c == '\0' */
return (NULL != strchr(removableChars, c));
}
Cheers
Michael
--
E-Mail: Mine is an /at/ gmx /dot/ de address.
.
- Follow-Ups:
- Re: K&R exercise 1-18
- From: Joe Van Dyk
- Re: K&R exercise 1-18
- From: Eric
- Re: K&R exercise 1-18
- References:
- K&R exercise 1-18
- From: Eric
- K&R exercise 1-18
- Prev by Date: Re: x=(x=5,11)
- Next by Date: Re: Malloc for array of string
- Previous by thread: Re: K&R exercise 1-18
- Next by thread: Re: K&R exercise 1-18
- Index(es):
Relevant Pages
|
