Re: K&R exercise 1-18

Michael Mair wrote:

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

That was an oversight, I will add them for future questions.


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.

I really appreciate you taking the time to help out. I know I have a lot of
hard work ahead to get somewhat proficient at this.

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

Thanks for making that clear. I think my confusion with keeping this
straight in my mind contributed to the overwrite of the last character when
a line exceeded the maximum length.


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.

I originally had parameter names the same but I was getting compiler
warnings about a shadow parameter, variable or something (I can't remember
exactly). I just tried to reproduce what the warning was but was
unsuccessful. I had interpreted it to mean that the parameter name in the
function definition should not be the same as the name of the variable that
the function was using. Obviously that was not correct and I wish I had
saved that message.

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

I look forward to pointer is chapter 5. :)


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.

I should have written that way. I had seen it mentioned many times in this
group but I was using K&R examples which are not even using "int" and
forgot about the (void). It does make it more clear that there are not any
function parameters.

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

I didn't realize that. I had "assumed" I was assigning line[] a value by
adding [MAXIMUM}. I have a long way to go before I will understand the
differences in the standards but maybe someday ...


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.

I wonder if this might be why I was getting segmentation faults when I was
testing earlier versions of the code. Remembering line[0] as the first
character and line[999] as the end of the array was more difficult than I
thought even though I read warnings about this many times. :)


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.

I used the "if(c == EOF)" loop because my earlier version would not print
out the last line if it did not have a newline after it. At first I added
a newline to it and then used the remblank() function. It worked but it
added a newline to the end of the output which I a first thought was good
but then decided that I should not have it add any extra characters. It
did make it look goofy in the terminal display without the extra
newline. :)


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

Good point and thanks.


/* 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'.


Thanks, I was thinking that a blank line would have at least an array length
of 2, the '\n' and the '\0', but got the actual length and the index count
confused. It must have been bad luck on my part that the program handled
the blank lines correctly for me cause I would have never found this error.

{
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

Thank you very much with all your help, Michael.

Eric
--
Remove "all the spam" to reply.
.

Relevant Pages

  • Re: Pointer
    ... require in input a pointer char: ... unsafe public static extern int OpenFile; ... What is it pointing to exactly, is it pointing to a "Unicode character" array or is it pointing to a "Single byte character" array or is it pointing to something else? ...
    (microsoft.public.dotnet.languages.csharp)
  • Re: Pointer
    ... require in input a pointer char: ... unsafe public static extern int OpenFile; ... What is it pointing to exactly, is it pointing to a "Unicode character" array or is it ...
    (microsoft.public.dotnet.languages.csharp)
  • Re: socket communication: send & receive doesnt work right
    ... Maybe start from sending simple byte array to check what happen with byte ... I did a test of sending two doubles: ... public void send_doubles(double vals, int len) throws IOException ... char *result; ...
    (microsoft.public.win32.programmer.networks)
  • Re: Memory Allocation Problem, please help
    ... typedef struct word_tag{ ... array is not an array. ... static int total_word_count; ... static int word_index(const char *word); ...
    (comp.lang.c)
  • Re: Write to file
    ... You can read the file one character at a time. ... compared to the current char. ... Once the (lgh - 1)th char ... void initnext(int *next, const char *id, int lgh) ...
    (comp.lang.c)