String copying not working? 
Author Message
 String copying not working?

Well here is my problem...
I read a string from stdin (statically allocated), and I copy
the string read into memory for saving, but part of the string
doesn't get copied. Why? It copies the beginnine and end, but
leaves blanks where there should be characters in the mid section.

I have my mainline looping around producing a menu and getting
menu options. One of the options is to read a string from the
user, which then gets passed to a function for storing.

i.e. In mainline somewhere, after choosing the option...
fgets(FilterString, MAX_FILTER_COMMAND_LENGTH, stdin);
FilterString[strlen(FilterString)-1] = '\0';
initPDUFilter (
        &PDUFilter[NumFilters],
        FilterString,
        (pcap_handler)QueueingFunction
);

As you can see, I get rid of the newline at the end of the string
before passing it to the function.

Inside the initPDUFilter function, i do this...

int initPDUFilter (ThreadedFilter_t* ThreadedPDUFilter, char*
FilterCommand,
                pcap_handler QueueingFunction) {

        StringLength = strlen(FilterCommand) + 1;
        fprintf(stderr, "Filter is %s\n", FilterCommand);

        ThreadedPDUFilter->Command = (char*)
malloc(sizeof(char)*StringLength);
        memset(ThreadedPDUFilter->Command, ' ', sizeof(char) *
StringLength);
        strcpy(ThreadedPDUFilter->Command, FilterCommand);
        fprintf(stderr, "Initialized PDU Filter %s\n",
ThreadedPDUFilter->Command);
        return 0;

Quote:
}

If i input the string from the keryboard as "src host
123.7.192.132<enter>"
i get output...
Filter is src host 123.7.192.132
Initialized PDU Filter src host.7.192.132

Why is not copying the 123?
I've tried using strcpy, strncpy, memcpy and all produce the same
results...
It's weird, cause I print the origianl string in the function and it is
OK.
Copying it tho isn't...

Thanks
Byron



Sat, 22 Mar 2003 14:43:41 GMT  
 String copying not working?

Quote:

> I have my mainline looping around producing a menu and getting
> menu options. One of the options is to read a string from the
> user, which then gets passed to a function for storing.

> i.e. In mainline somewhere, after choosing the option...
> fgets(FilterString, MAX_FILTER_COMMAND_LENGTH, stdin);
> FilterString[strlen(FilterString)-1] = '\0';
> initPDUFilter (
>         &PDUFilter[NumFilters],
>         FilterString,
>         (pcap_handler)QueueingFunction
> );

> As you can see, I get rid of the newline at the end of the string
> before passing it to the function.

Only if there _is_ a newline. If you enter more than
MAX_FILTER_COMMAND_LENGTH characters (including newline), you get rid of
another character, and you won't read the newline until the next input.

Quote:
> Inside the initPDUFilter function, i do this...

> int initPDUFilter (ThreadedFilter_t* ThreadedPDUFilter, char*
> FilterCommand,
>                 pcap_handler QueueingFunction) {

It would be nice to know what a ThreadedFilter_t and a pcap_handler
_are_, but...

Quote:
>         StringLength = strlen(FilterCommand) + 1;
>         fprintf(stderr, "Filter is %s\n", FilterCommand);

>         ThreadedPDUFilter->Command = (char*)
> malloc(sizeof(char)*StringLength);

- Don't cast the result from malloc(). You may well be masking errors,
  such as not having #included <stdlib.h>, and it doesn't do anything
  useful.
- sizeof(char) is always 1, so you can get rid of it. You might want to
  replace it with sizeof *ThreadedPDUFilter->Command, though.
- Always check if a malloc() succeeded.

Quote:
>         memset(ThreadedPDUFilter->Command, ' ', sizeof(char) *
> StringLength);

I don't understand why you do this, since you immediately overwrite the
memory again.

Quote:
>         strcpy(ThreadedPDUFilter->Command, FilterCommand);
>         fprintf(stderr, "Initialized PDU Filter %s\n",
> ThreadedPDUFilter->Command);
>         return 0;
> }

> If i input the string from the keryboard as "src host
> 123.7.192.132<enter>"
> i get output...
> Filter is src host 123.7.192.132
> Initialized PDU Filter src host.7.192.132

That's very strange indeed. Does it do this with _every_ string you
enter, or just with this one? I can't see anything above that could
explain this, except that if your malloc() call is incorrect for one
reason or another that invokes undefined behaviour (and writing to
memory that wasn't actually successfully allocated does, for example),
the implementation is basically allowed to do whatever it wants.

Richard



Sat, 22 Mar 2003 03:00:00 GMT  
 String copying not working?
Thanks for the help Richard. Much appreciated.
I have read what you've said, and taken that into consideration, had
another play with the code. I found out that the problem is that
memory allocation as you said, but *NOT* in that it fails.

It succeeds in allocating the memory, but when allocating this
memory, it corrupts the input sting to the function. That is really
strange..?

I printf the input string before the malloc, and no probs...
I printf the input string after the malloc, but alas, corruption.
NOTE: Read end of message

Quote:


> > i.e. In mainline somewhere, after choosing the option...
> > fgets(FilterString, MAX_FILTER_COMMAND_LENGTH, stdin);
> > FilterString[strlen(FilterString)-1] = '\0';
> > initPDUFilter (
> >         &PDUFilter[NumFilters],
> >         FilterString,
> >         (pcap_handler)QueueingFunction
> > );

> > As you can see, I get rid of the newline at the end of the string
> > before passing it to the function.

> Only if there _is_ a newline. If you enter more than
> MAX_FILTER_COMMAND_LENGTH characters (including newline), you get rid of
> another character, and you won't read the newline until the next input.

k. MAX_FILTER_COMMAND_LENGTH is #define'd to be 128, and I am using a
much
smaller string, but point taken. This is partly why I have made the max
string length so big (bad solution tho, so thanx).

Quote:
> > Inside the initPDUFilter function, i do this...

> > int initPDUFilter (ThreadedFilter_t* ThreadedPDUFilter, char*
> > FilterCommand,
> >                 pcap_handler QueueingFunction) {

> It would be nice to know what a ThreadedFilter_t and a pcap_handler
> _are_, but...

Well, pcap_handler is a function pointer. It is my ququeing function
that I pass to the pcap library for when a PDU is successfuly filtered.
The ThreadedFilter_t is my own ADT. It looks like this...

struct ThreadedFilter {
        /* EXTERNAL INFORMATION */
        pcap_handler    QueueingFunction;
        char*           Command; /* The filter command in BPF syntax */
        pcap_t*         Descriptor; /* Network Descriptor */

        /* INTERNAL INFORMATION */
        Queue_t         PDUQueue; /* Where the PDU's are buffered */
        Filter_t        Filter; /* Internal pcap data */

        /* Information associated with the thread conrtolling the filtering */
        pthread_t       ThreadID;       /* Unique thread identifier */
        Callback        ThreadFunction; /* Executed by pthread_create */
        int             ExecutionState; /* ON or OFF */
        pthread_mutex_t Mutex;          /* Synchronization variable */

Quote:
};

typedef struct ThreadedFilter ThreadedFilter_t;

Quote:
> >         StringLength = strlen(FilterCommand) + 1;
> >         fprintf(stderr, "Filter is %s\n", FilterCommand);

> >         ThreadedPDUFilter->Command = (char*)
> > malloc(sizeof(char)*StringLength);

> - sizeof(char) is always 1, so you can get rid of it. You might want to
>   replace it with sizeof *ThreadedPDUFilter->Command, though.

I'm not sure if sizeof(char) is guarnteed to be 1 tho. I'm trying to be
as
'correct' as possible and implement portable and scalable code. eg: The
character set may require more than 1 byte (unicode, chinese, etc..). I
also seems to remember a few machines that still don't have 1 byte
charsets.
eg: cray's, pdp's, etc...
It also states in the FAQ that sometimes a char may be implemented as an
int, in which case...
Anyway, any affects seen from misuse of that form have not appeared in
my
output. If this were the case, I would have not enough or too much space
allocated for my string.

Quote:
> - Always check if a malloc() succeeded.

k, I have done that, and yes, it does succeed

Quote:
> >         memset(ThreadedPDUFilter->Command, ' ', sizeof(char) *
> > StringLength);

> I don't understand why you do this, since you immediately overwrite the
> memory again.

yeah, a leftover from hacking away trying to check different stuff.
Also trying to be 'nice'.

Quote:
> >         strcpy(ThreadedPDUFilter->Command, FilterCommand);
> >         fprintf(stderr, "Initialized PDU Filter %s\n",
> > ThreadedPDUFilter->Command);
> >         return 0;
> > }

> > If i input the string from the keryboard as "src host
> > 123.7.192.132<enter>"
> > i get output...
> > Filter is src host 123.7.192.132
> > Initialized PDU Filter src host.7.192.132

> That's very strange indeed. Does it do this with _every_ string you
> enter, or just with this one?

It happens with all strings greater than a certain length. The
corruption
occurs in the 8, 9, 10 and 11th positions of the string (0 based), so
any string that has 9 or more characters gets corrupted.


Sat, 22 Mar 2003 03:00:00 GMT  
 String copying not working?
k, problem solved.
What a stupid mistake :(
The statically allocated tring in the mainline wasn't big
enough to hold MAX_FILTER_COMMAND_LENGTH characters. It had
some other (incorrect) #define for its size.

I still find it strange however that fgets didn't cause a
{*filter*}tation fault when storing the string read, as it was
bigger than the buffer given to it.

This only problem that arose was the 'corruption' of the
original string when malloc was called. hmm...something
implementation dependant in the C library? I'd have a wild
guess that this would have turned out differently had i been
using another system.

Byron



Sat, 22 Mar 2003 03:00:00 GMT  
 String copying not working?


Quote:
> > > malloc(sizeof(char)*StringLength);

> > - sizeof(char) is always 1, so you can get rid of it. You might want
> > to replace it with sizeof *ThreadedPDUFilter->Command, though.

> I'm not sure if sizeof(char) is guarnteed to be 1 tho.

it *is*. It is defined to be 1 by the Standard. A char may be more than
8 bits in size but sizeof(char) always yeilds 1. Always.

Quote:
> I'm trying to be as 'correct' as possible and implement portable and
> scalable code.

excellent!  :-)
You can rely on sizeof(char) to remain the same on all platforms.

Quote:
> eg: The character set may require more than 1 byte (unicode, chinese,
> etc..).

the standard defines byte and char to be the same thing,
counter-intuitive though this may be. A byte may not necessarily be
8-bits though. Comms people avoid this confusion by referring to an
8-bit quantity as an octet.

Quote:
> I also seems to remember a few machines that still don't have 1 byte
> charsets. eg: cray's, pdp's, etc...

their C compilers will have 1-byte chars though.

Quote:
> It also states in the FAQ that sometimes a char may be implemented as
> an int, in which case...

I think you need to re-read the FAQ. If a char is the same as an int on
a particular C implementation then:-

   sizeof(int) == sizeof(char) == 1

must be true, which is quite possible and quite legal. Apparantly some
DSPs have C compilers whre everything is 32-bits.

Quote:
> Anyway, any affects seen from misuse of that form have not appeared in
> my output. If this were the case, I would have not enough or too much
> space allocated for my string.

true what you do is harmless, but it indicates that you don't really
understand the releationships between the fundamental types. You
probably think a byte is 8-bits and a char is different from a byte.

--
Abuse of casting leads to abuse of the type system
leads to sloppy programming
leads to unreliably, even undefined, behaviour.
And that is the path to the dark side....
                         Richard Bos/John Hascall

Sent via Deja.com http://www.deja.com/
Before you buy.



Sat, 22 Mar 2003 03:00:00 GMT  
 String copying not working?

Quote:

> What a stupid mistake :(

I hardly ever make any other kind.

Quote:
> The statically allocated tring in the mainline wasn't big
> enough to hold MAX_FILTER_COMMAND_LENGTH characters. It had
> some other (incorrect) #define for its size.

> I still find it strange however that fgets didn't cause a
> {*filter*}tation fault when storing the string read, as it was
> bigger than the buffer given to it.

Well, that's undefined behaviour for you. Anything may happen.
Presumably, the area just beyond your string was writable, but belonged
to another variable. When you write to the other variable, you
accidentally also write to your too-long string.

BTW, a cheap and easy, but well-working, way of getting around this is
to replace
  fgets(FilterString, MAX_FILTER_COMMAND_LENGTH, stdin);
by
  fgets(FilterString, sizeof FilterString, stdin);

Note, though, that this only works if a declaration of FilterString as
an array is in scope; if you've passed it to a function and call fgets()
from that function, for example, you haven't got the array anymore,
you've got a pointer to its first element, which you can _usually_ treat
as if it were an array, but not always; in particular, in this case, its
sizeof is different.

Richard



Sat, 22 Mar 2003 03:00:00 GMT  
 String copying not working?

Quote:
>The statically allocated tring in the mainline wasn't big
>enough to hold MAX_FILTER_COMMAND_LENGTH characters. It had
>some other (incorrect) #define for its size.

>I still find it strange however that fgets didn't cause a
>{*filter*}tation fault when storing the string read, as it was
>bigger than the buffer given to it.

Nothing strange here, unless the buffer was really allocated on the
edge of your program's data segment.  Most likely, you've ended up
overwriting some other of your program's data.  And that could explain
any kind of strange behaviour.

Quote:
>This only problem that arose was the 'corruption' of the
>original string when malloc was called. hmm...something
>implementation dependant in the C library? I'd have a wild
>guess that this would have turned out differently had i been
>using another system.

You've been lucky to use a system where your mistake cause an immediately
visible problem, rather than having your program working by accident.

The lesson to be learned is that the *correct* way to call fgets is:

    rc = fgets(buffer, sizeof buffer, stream);

assuming, of course, that buffer is an array and not some kind of pointer.

Another lesson: whenever asking help about a program, don't post disparate
bits of its code.  Write a *minimal* program reproducing your problem and
post it without omitting anything, explaining what you expected it to do
and what it actually did.  Chances are that, while doing that, you'll
discover the bug yourself.

Dan
--
Dan Pop
CERN, IT Division

Mail:  CERN - IT, Bat. 31 1-014, CH-1211 Geneve 23, Switzerland



Sat, 22 Mar 2003 03:00:00 GMT  
 String copying not working?
Thanks for all the help, much appreciated.
I've certainly learnt some good lessons through this exercise and
am a wiser (and therefore better :) person now. hehehe

The advice for producing a minimal program first will be adopted
as a new law in my religion.

I had a misconception about chars as well. Thanks for pointing that
out. I should've know better, considering i'm meant to be a comms
person :)

Byron Hammond



Sun, 23 Mar 2003 01:59:46 GMT  
 
 [ 8 post ] 

 Relevant Pages 

1. Setting not Null field to ""(empty string) does not work with CRecordset

2. Setting not Null field to ""(empty string) does not work with CRecordset

3. Number of Copies Not Working

4. Subclassed copy constructor not working

5. Copy/Paste using Ctrl+C/Ctrl+V does not work in CHtmlView Class

6. Copy/Paste via keyboard not working in FormView

7. Rich Edit cut/copy does not work?

8. copy does not work

9. I do not understand this string copy function

10. C++/ATL/ADO - Intellisense not working (statement completion options) not working

11. String comparation not working

12. automaticly showing methods (works for string, not stringstream)

 

 
Powered by phpBB® Forum Software