Wrong output - eliminate spaces in sentence.

  • Thread starter msundaram.visvanathan
  • Start date
M

msundaram.visvanathan

Hi,
i tried to write this small code in C which would:

Input :a b c d
Output:abcd


code:
#include<stdio.h>

void main()
{
char t[]="a b c d";
char *q,*r;
int count=0;

q=t;
r=t;
printf("\ninput %s",t);
while(*q!='\0')
{

if(*q!=' ' && r!=q )
{
*r=*q;
*q=' ';
r=q;
continue;
}
else if(*q==' ')
{
q++;
continue;
}
else if(*q!=' ')
{
q++;
r++;
}
}

printf("\noutput %s",t);

}


With this I get :
input:a b c d
output:abc d


please provide your comments on what could be wrong.

Sundar.
 
V

Vladimir Oka

Hi,
i tried to write this small code in C which would:

Input :a b c d
Output:abcd


code:
#include<stdio.h>

void main()

All bets are off -- `main()` should return an `int`:

int main(void)
{
char t[]="a b c d";
char *q,*r;
int count=0;

You're not using `count` anywhere.
q=t;
r=t;
printf("\ninput %s",t);
while(*q!='\0')
{

if(*q!=' ' && r!=q )
{
*r=*q;
*q=' ';
r=q;

Here's your problem. It should be:

r++;

Have you tried this with pen(cil) and paper?
continue;

This `continue;` is superfluous.
}
else if(*q==' ')
{
q++;
continue;

This one as well.
}
else if(*q!=' ')
{
q++;
r++;
}
}

printf("\noutput %s",t);

Not terminating `printf()` with '\n' may lead to no output at all (you
could use `fflush(stdout)` as well).

Spacing is atrocious, especially on Usenet.
 
C

CBFalconer

i tried to write this small code in C which would:

Input :a b c d
Output:abcd

code:
#include<stdio.h>

void main()

main returns int, always.
{
char t[]="a b c d";
char *q,*r;

Use more descriptive names.
int count=0;

don't include useless unused variables
q=t;
r=t;
printf("\ninput %s",t);

The \n goes at the end, to cause the results to display.
while(*q!='\0')
{

Now you are off the track ... snipped tortuous logic, probably
caused by unsuitable names.
}

printf("\noutput %s",t);

Same comment. Main returns an int, do so.
}

With this I get :
input:a b c d
output:abc d

please provide your comments on what could be wrong.

Try this:

#include<stdio.h>

int main(void)
{
char t[]=" a b c d ";
char *source, *dest;

source = dest = t;
printf("input \"%s\"\n",t); /* note quotes to delimit blanks */

do {
while (' ' == *source) source++;
*dest++ = *source++;
} while (*source);
*dest = *source;

printf("output \"%s\"\n",t);
return 0;
}
 
D

deepak

What Oka said is correct.
You should use r++ instead of r=q.
If you give characters more than length 4, it 'll show same type of
error.

Deepak.

Vladimir said:
Hi,
i tried to write this small code in C which would:

Input :a b c d
Output:abcd


code:
#include<stdio.h>

void main()

All bets are off -- `main()` should return an `int`:

int main(void)
{
char t[]="a b c d";
char *q,*r;
int count=0;

You're not using `count` anywhere.
q=t;
r=t;
printf("\ninput %s",t);
while(*q!='\0')
{

if(*q!=' ' && r!=q )
{
*r=*q;
*q=' ';
r=q;

Here's your problem. It should be:

r++;

Have you tried this with pen(cil) and paper?
continue;

This `continue;` is superfluous.
}
else if(*q==' ')
{
q++;
continue;

This one as well.
}
else if(*q!=' ')
{
q++;
r++;
}
}

printf("\noutput %s",t);

Not terminating `printf()` with '\n' may lead to no output at all (you
could use `fflush(stdout)` as well).

Spacing is atrocious, especially on Usenet.
 
N

Nelu

deepak said:
What Oka said is correct.
You should use r++ instead of r=q.
If you give characters more than length 4, it 'll show same type of
error.

Plea don't top post. It makes it hard to read.

Thank you.
 
S

spibou

CBFalconer said:
i tried to write this small code in C which would:

Input :a b c d
Output:abcd

code:
#include<stdio.h>

void main()

main returns int, always.
{
char t[]="a b c d";
char *q,*r;

Use more descriptive names.
int count=0;

don't include useless unused variables
q=t;
r=t;
printf("\ninput %s",t);

The \n goes at the end, to cause the results to display.
while(*q!='\0')
{

Now you are off the track ... snipped tortuous logic, probably
caused by unsuitable names.
}

printf("\noutput %s",t);

Same comment. Main returns an int, do so.
}

With this I get :
input:a b c d
output:abc d

please provide your comments on what could be wrong.

Try this:

#include<stdio.h>

int main(void)
{
char t[]=" a b c d ";
char *source, *dest;

source = dest = t;
printf("input \"%s\"\n",t); /* note quotes to delimit blanks */

do {
while (' ' == *source) source++;
*dest++ = *source++;
} while (*source);
*dest = *source;

printf("output \"%s\"\n",t);
return 0;
}

I'm not sure if the code above is meant to test the opening
poster's dilligence but I think that the mistake may be too
subtle for a beginner and it won't necessarily be revealed by
testing the programme either so I'll spill (some of) the beans.
Scroll below for a hint.













What happens if after the line
while (' ' == *source) source++;
source points to the terminating byte ? Then
source may boldly go where no pointer has
gone before !

Spiros Bousbouras
 
C

CBFalconer

CBFalconer wrote:
.... snip ...
Try this:

#include<stdio.h>

int main(void)
{
char t[]=" a b c d ";
char *source, *dest;

source = dest = t;
printf("input \"%s\"\n",t); /* note quotes to delimit blanks */

do {
while (' ' == *source) source++;
*dest++ = *source++;
} while (*source);
*dest = *source;

printf("output \"%s\"\n",t);
return 0;
}

I'm not sure if the code above is meant to test the opening
poster's dilligence but I think that the mistake may be too
subtle for a beginner and it won't necessarily be revealed by
testing the programme either so I'll spill (some of) the beans.
Scroll below for a hint.

What happens if after the line
while (' ' == *source) source++;
source points to the terminating byte ? Then source may boldly
go where no pointer has gone before !

You are right. I belatedly noticed it earlier, and prepared the
following for posting when my previous answer showed up.

#include <stdio.h>

int main(int argc, char* *argv)
{
char *source, *dest;

if (2 <= argc) {
source = dest = argv[1];
printf("input \"%s\"\n", argv[1]);

do {
while (' ' == *source) source++;
*dest++ = *source++; /* 13 */
} while (*source); /* 14 */
*dest = *source; /* 15 */

printf("output \"%s\"\n", argv[1]);
}
return 0;
}

This has an evil bug in it. If, in line 13, *source is '\0' and
that is the last valid location in argv[1], then line 14 is
dereferencing past the last location, and UB is present. I see
no simple way of preventing that without tortuous tests. Yet I
have been unable to trigger the bug with the above code!!

Am I missing something? Run with:

prog " a b c d " whatever

and vary the count of final blanks in argv[1].
 
S

spibou

CBFalconer said:
#include <stdio.h>

int main(int argc, char* *argv)
{
char *source, *dest;

if (2 <= argc) {
source = dest = argv[1];
printf("input \"%s\"\n", argv[1]);

do {
while (' ' == *source) source++;
*dest++ = *source++; /* 13 */
} while (*source); /* 14 */
*dest = *source; /* 15 */

printf("output \"%s\"\n", argv[1]);
}
return 0;
}

This has an evil bug in it. If, in line 13, *source is '\0' and
that is the last valid location in argv[1], then line 14 is
dereferencing past the last location, and UB is present. I see
no simple way of preventing that without tortuous tests.

I don't think it's that complicated. Just change line 13 to
*dest = *source; /* 13 */
if (*source == 0) break; /* 14 */
dest++ ; source++ ; /* 15 */
and you can also change while (*source); to while(1)
since the test is not needed anymore.
Yet I
have been unable to trigger the bug with the above code!!

What do you mean trigger the bug ? What did you expect to
observe ?
Am I missing something? Run with:

prog " a b c d " whatever

and vary the count of final blanks in argv[1].

I did. I also added one extra line to check where source
points to. If the input ends with blank(s) then source happily
continues after it. But on Solaris what follows is the list of
environmental variables so when source reaches the NUL after
the first env. variable the programme terminates normally.
 
C

CBFalconer

CBFalconer said:
#include <stdio.h>

int main(int argc, char* *argv)
{
char *source, *dest;

if (2 <= argc) {
source = dest = argv[1];
printf("input \"%s\"\n", argv[1]);

do {
while (' ' == *source) source++;
*dest++ = *source++; /* 13 */
} while (*source); /* 14 */
*dest = *source; /* 15 */

printf("output \"%s\"\n", argv[1]);
}
return 0;
}

This has an evil bug in it. If, in line 13, *source is '\0' and
that is the last valid location in argv[1], then line 14 is
dereferencing past the last location, and UB is present. I see
no simple way of preventing that without tortuous tests.

I don't think it's that complicated. Just change line 13 to
*dest = *source; /* 13 */
if (*source == 0) break; /* 14 */
dest++ ; source++ ; /* 15 */
and you can also change while (*source); to while(1)
since the test is not needed anymore.
Yet I
have been unable to trigger the bug with the above code!!

What do you mean trigger the bug ? What did you expect to
observe ?

I was expecting the bug to effectively extend the source string, so
that extra garbage would be added. Not observed. I am much too
lazy to apply gdb and look at the details. The whole thing is a
curiosity, not a really useful subroutine. And I dislike break, in
general I prefer an honest goto where the destination is clearly
marked.
 
P

pete

Hi,
i tried to write this small code in C which would:

Input :a b c d
Output:abcd

The squeeze function from K&R section 2.8, makes this easy.

/* BEGIN new.c */

#include <stdio.h>

void squeeze(char *s, int c);

int main(void)
{
char t[] = "a b c d";

squeeze(t, ' ');
puts(t);
return 0;
}

void squeeze(char *s, int c)
{
size_t i,j;

for (i = j = 0; s != '\0'; ++i) {
if (s != c) {
s[j++] = s;
}
}
s[j] = '\0';
}

/* END new.c */
 
S

spibou

CBFalconer said:
(e-mail address removed) wrote:

I was expecting the bug to effectively extend the source string, so
that extra garbage would be added. Not observed. I am much too
lazy to apply gdb and look at the details. The whole thing is a
curiosity, not a really useful subroutine. And I dislike break, in
general I prefer an honest goto where the destination is clearly
marked.

The extra garbage was probably added. But since the NUL byte was
put inside dest before the garbage , the final printf does not print
the
garbage.
 
C

CBFalconer

The extra garbage was probably added. But since the NUL byte was
put inside dest before the garbage , the final printf does not print
the garbage.

Aha - sounds reasonable. So, making some assumptions, printing out
argv[2] at the end should show it up!. A sneaky evil bug.
 
C

CBFalconer

pete said:
i tried to write this small code in C which would:

Input :a b c d
Output:abcd

The squeeze function from K&R section 2.8, makes this easy.
.... snip ...

void squeeze(char *s, int c)
{
size_t i,j;

for (i = j = 0; s != '\0'; ++i) {
if (s != c) {
s[j++] = s;
}
}
s[j] = '\0';
}

/* END new.c */


Yes, that avoids all the nonsense I got myself into.
 
R

Richard G. Riley

CBFalconer said:
I was expecting the bug to effectively extend the source string, so
that extra garbage would be added. Not observed. I am much too
lazy to apply gdb and look at the details. The whole thing is a

Debuggers are for people who dont understand the code as I recall from
your previous posts.

See http://tinyurl.com/reztp for how real men dont use debuggers and how
printfs are enough even in huge multilayered, multi-system legacy based
systems.... Hell, Rod has only resorted to using a debugger twice in his
life ....

Clearly you are doing something very wrong .... But good to see that
now you recognise how a debugger is indeed useful to inspect and tweak
runtime values in order to find silly little bugs such as those
exhibited earlier in the thread. And all with no changes to the code and
second guessing where to put those pesky printfs ...
 
C

CBFalconer

CBFalconer said:
The extra garbage was probably added. But since the NUL byte was
put inside dest before the garbage , the final printf does not print
the garbage.

Aha - sounds reasonable. So, making some assumptions, printing out
argv[2] at the end should show it up!. A sneaky evil bug.

Expanding, the bug was caused by the erroneous nesting of loop
operations. My original nested a loop with side effects inside a
scanning loop. Simply making the inner operation a pure
conditional eliminates any bugs:

while (*source) {
if (' ' != *source) *dest++ = *source;
source++;
}
*dest = '\0';

A lesson in basic design.
 
A

av

I don't think it's that complicated. Just change line 13 to
*dest = *source; /* 13 */
if (*source == 0) break; /* 14 */
dest++ ; source++ ; /* 15 */
and you can also change while (*source); to while(1)
since the test is not needed anymore.

yes

#include<stdio.h>

#define W while
#define R return
#define P printf

int levas(char* o, int c)
{char *d=o, *pc=o;
W(1){W(*o==c) ++o;
if((*d=*o)==0) break;
++o, ++d;
}
R d-pc;
}


int main(void)
{
char t[]=" a b c d ";
char *source, *dest;
int len;

source = dest = t;
printf("input \"%s\"\n",t); /* note quotes to delimit blanks */
len=levas(t, ' ');

printf("output \"%s\" len=%d\n", t, len);
return 0;
}
 
A

av

Hi,
i tried to write this small code in C which would:

Input :a b c d
Output:abcd

for that routine i prefer assembly

; nasmw -f obj this_file.asm
; bcc32 this_file.obj

section _DATA public align=4 class=DATA use32

global _levac

section _TEXT public align=1 class=CODE use32

; int leva(char* o, int c)
; s=0j, 4i, 8ra, 12@o, 16@c
_levac:
push esi
push edi
%define @o [esp+12]
%define @c [esp+16]
mov esi, @o
mov edi, @o
mov ah, @c
jmp short .c2
..c0:
inc edi
..c1:
inc esi
..c2:
cmp byte[esi], ah
je .c1
mov al, [esi]
mov [edi], al
cmp al, 0
jne .c0
mov eax, edi
sub eax, @o
%undef @o
%undef @c
pop edi
pop esi
ret

19 instrucions
agains 26 of a tipical c implementation

;
;int levas(char* o, int c)
;
push ebp
mov ebp,esp
push ebx
push esi
mov edx,dword ptr [ebp+12]
mov eax,dword ptr [ebp+8]
;
; {char *d=o, *pc=o;
;
?live1@160: ; EAX = o, EDX = c
@10:
mov ecx,eax
mov esi,eax
;
; G c0;
;
?live1@176: ; EAX = o, EDX = c, ECX = d, ESI = pc
jmp short @14
;
; do {++o, ++d;
;
@12:
inc eax
inc ecx
jmp short @14
;
; c0:; W(*o==c) ++o;
;
@13:
inc eax
@14:
movsx ebx,byte ptr [eax]
cmp edx,ebx
je short @13
;
; }W(*d=*o);
;
mov bl,byte ptr [eax]
mov byte ptr [ecx],bl
test bl,bl
jne short @12
;
; R d-pc;
;
?live1@240: ; ECX = d, ESI = pc
mov eax,ecx
sub eax,esi
;
; }
;
?live1@256: ;
@17:
@16:
pop esi
pop ebx
pop ebp
ret
_levas endp
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top