receiving-server

Hi Begin,

I'm more concerned with the code around line 53.

The program reads an integer from the socket (size_to_read) which is the file length.  You then try to to read that many bytes from the network into buffer pch.  There is no testing to ensure that (size_to_read) is smaller than the buffer.


Kent

Let me review code around line 53. tks.

just updated. is the code on right track now? I just want to focus on the logic first before i run and debug. tks.
server.c

Before submitting, you need to test. Otherwise, we are simply giving you pointers without you doing the real work. Be careful about using sizeof, which is the number of bytes that an object takes up.

>> unsigned int size_to_read
>> if(n < sizeof (size_to_read))

sizeof (size_to_read) is probably 4 since size_to_read is an unsigned int.

Does it make sense that your general receiveMessage() function tests for n < 4?

You probably should include your latest client code, since this is what I have from previous question:
 sendMessage():

   
     char pch[BUFFER_LEN]= {'\0'};
    ...
     n =send (sock, pch, a.size(), 0);

    if(n != sizeof(pch))
    {	
        error("Error in sending data buffer");
    }

Select allOpen in new window

Then sizeof(pch) is BUFFER_LEN. For some messages, n < BUFFER_LEN, and you exit. So, assuming that you ran this code, I must have the wrong client code.

>> I just want to focus on the logic first before i run and debug.
I think you should focus on the logic, and when satsified, you should run and debug. If you then are stuck, you can ask a question.

Hi Begin,

At some point you'll want to change the way that you compute how long the length field will be.

  sizeof (int)

As long as you control the source code and both ends are compiled on equal systems, this works fine.  But if you ever "release" the code so that anyone can compile the client to work with your server you'll run the risk of the client being run on a 16 or (more likely) 64-bit system.  That will create a mismatch that just won't work.

But for now, sizeof(int) will work just fine!


Kent

In the client, at send data inside sendMessage(), we need to check the size of data sending with size of the send() return in order to make sure it sends enough size of data. so i think the code should like this:
       n =send (sock, pch, a.size(), 0);
      if(n != a.size())
      {      
            error("Error in sending data buffer");
      }

Am i right?

hihi. So similar in server side,

      n = read( sock, pch, sizeof(pch));
      if(n < sizeof (pch))
      {      
            error("Error in receiving data buffer");
      }
Am i right this time?

I like it.  :)

tks a lot. I can start debugging now. However, i need to go to work now. I gonna be back 9 hrs later. sorry .SO exhausted . every i had code in my dream.

>> char  pch[BUFFER_LEN]
then  sizeof(pch) is BUFFER_LEN

>> n = read( sock, pch, sizeof(pch));
For shorter messages the number of bytes read will be less than BUFFER_LEN, but then

>>  if(n < sizeof (pch))
becomes
       if(n < BUFFER_LEN )
              error()

and you get an error.

If you take 1 minute to run your program in debugger, you will find the answers to your questions.

begintolearn,

you have to read message1, message2, message3.

any message contains of size_to_read (integer), message_id (integer), textdata.

only the textdata has variable size which you know because it was passed in size_to_read.

so you have 3 kinds of reading

   n = read(sock, &size_to_read, sizeof(int));

   n = read(sock, &message_id, sizeof(int));

   n = read(sock, pch, size_to_read);

and you not necessarily have to expect more or less bytes for any of the reads (cause you've written both client and server yourself).

but the tcpip message is not guaranteed to be received in total. you could get only parts of those. that means the n can be less than the expected length and you would get the remainder with a second or even third read.

i posted a code snippet of how to handle that in one of your previous q. you should use that at least for the message 3 cause a 2k buffer is not granted to get received in total.

Sara

when reading message 3 you should add all received size_to_read and check that sum against the filesize you got with message 2. that will tell you when you can stop reading message 3. same the number of files got by message 1 will tell you when to stop to read new files.

you should know that if you do more reads than for that what was sent you finally will be blocked (hang).

Sara

I dont have computer aces now.pls tell me any hiden error so I xan work on them once  I get home.tks

When you get home, I think I'll be around to see if you have questions on your code.

please post your code (both client and server) again here in the thread cause new participients may not  know where your latest sources can be received from.

Sara

I will post them soon once I get access to my pc.sorry

I just review all comments during breaktime,i can see the main issue is message3 data can vary.let me think a way to fix it.tks

Hi Begin,

On the server side, all you really need to do is keep track of a few things (status, etc.) and the entire code writes very small.

Check out the sample below.  It should be a good skeleton and is very easily extendable.


Kent

enum
{
  EXPECT_BLOCKTYPE,
  EXPECT_BLOCKLENGTH,
  EXPECT_DATA
};

#define BUFFERLENGTH 2048

  int Function;
  int ExpectedLength;
  int FullLength;
  int DataLength;
  int ReadLength;
  int BlockType;
  char *Buffer;

  Function = EXPECT_BLOCKTYPE;
  Buffer = (char*) malloc (BUFFERLENGTH);
  ExpectedLength = sizeof (int);
  while (1)
  {
    ReadLength = (sock, &Buffer, ExectedLength);
    if (ReadLength != ExpectedLength)
    {
      perror ("Read failure.");
      exit (1);
    }
    switch (Function)
    {
      case EXPECT_BLOCKTYPE:
        BlockType = *(int*)Buffer;
        fprintf (stderr, " blocktype: %d\n", BlockType);
        ++Function;
        break;
        
      case EXPECT_BLOCKLENGTH:
        FullLength = *(int*)Buffer;
        fprintf (stderr, " blocklength: %d\n", BlockLength);
        ++Function;
        break;
        
      case EXPECT_DATA:
        // Do something with the data
        fprintf (stderr, " readlength: %d\n", ReadLength);
        if (ReadLength == 0 && ExpectedLength == 0)
        {
          Function = EXPECT_BLOCKTYPE;
          ExpectedLength = sizeof (int);
          break;
        }
        ExpectedLength = FullLength > BLOCKLENGTH ? BLOCKLENGTH : FullLength;
        ExpectedLength -= FullLength;
        break;
      default:
        perror ("Block type unknown.");
        exit (1);
    }
  }

Select allOpen in new window

Just got home. While driving , i think i should use the length of buffer and multiply to size of each element in order to get size of data in message3. Am i right? Anyway let me marinate pork then come back in immediately. I apply multitasking now :)

here is my code up-to-date. It can transfer 5/6 files and those new files written to server have wrong size. on server terminal i got this error message:
  received wrong size_to_read: Success
Now i can start debugging process now :)
client.c
server.c

I have a few question need to clarify:
   + in client.c  around line 126,
              while ((result= fread (buffer, 1, BUFFER_SEND, fp) > 0))
 how can result is always equal to 1
   + when we deal with sizeof( buffer), should we minus 1 because of the NULL character. We need NULL character just for other functions to know where to terminate. However, the content of buffer, which need to write to file or using to increase nread, should exclude the last NULL character.

  Please advise.

CLIENT:
Line 126: >>  how can result is always equal to 1?
I believe I tried to help you on this here http://rdsrc.us/7RsjEk and and in here http://rdsrc.us/DkpnHB

>> when we deal with sizeof( buffer), should we minus 1 because of the NULL character
I found one case of sizeof( buffer) in the client:

Line 138: >> memset(buffer, '\0',sizeof(buffer)*sizeof(buffer[0])) ;
When trying to completely zero out buffer, all you need to do is:
                     memset(buffer, '\0', sizeof(buffer) ) ;
If your buffer ever became an array of long, then your original memset would overflow your buffer when you write 0's.
Right now, your buffer is an array of char, so sizeof(buffer[0]) is 1, so equivalent (for now).

>> should we minus 1 because of the NULL character
You have a buffer, so you may as well use the entire buffer. If you are sending a string, then why not transfer the null byte? Just remember that when you send a binary file, there are likely to be null bytes in the middle of a block, and likely that there will be no null byte at the end - that file data is not a c-style or c++ string. In previous question, I posted that you were convert your file read buffer to a string and then converting it back to a char array. Very inefficient, and if null bytes are in the middle, I wonder what the effect will be when converting to a string.

Don't forget that at the lower layer socket functionality, you are dealing with older libraries that are written in C style code (i.e., use char* and not C++ string). So you wrapper around this code should be also lower level to handle the binary files (and now your char buffer is not a c-style string - it is just an array of 8-bit integers). This lower level function will have to include the size of the file. In C++ you can define a function with the same name but having a string interface instead of a char* interface. Then in this string type function, you just call the lower level function making the adjustments to the lower level api.

I'll be back tomorrow (my timezone is 3 hours different than yours). I believe others start arriving shortly. So, to get their attention, create a new question.

the

    memset(buffer, '\0',sizeof(buffer)*sizeof(buffer[0]))

was my fault.

the sizeof already returns number of bytes. so the multiplication is wrong even if it doesn't matter for char arrays.

i mixed things up with the case where you need to compute the number of elements of an array. there you can do:

   int num_elements = sizeof(array)/sizeof(array[0]);

what would work for arrays of any type.

Sara

regarding the point whether the text buffer already should contain a terminating zero char or not, i only would like to say that buffers sent via sockets shouldn't have a maximum size 2k + 1 but 2k. an 'odd' size like 2k+1 may not generate problems in nowadays computing but it surely increases the probability that a buffer was sent in two parts instead of one. you should see that it doesn't make any problems to add a zero character at buffer[number_of_bytes_read] by the receiver granted that the buffer has that extra byte left.

the

   while ((result= fread (buffer, 1, BUFFER_SEND, fp) > 0))

is wrong. it needs to be

  while ((result= fread (buffer, 1, BUFFER_SEND, fp)) > 0)

only so the result will compared with 0. otherwise you have implicit parantheses like

    result= (fread (buffer, 1, BUFFER_SEND, fp) > 0);

and the result is always 1 (boolean) as long as the fread doesn't fail.

Sara





   

Sara

 

>> when we deal with sizeof( buffer), should we minus 1 because of the NULL character
When you said CLIENT, I thought you meant only the client. I took a look at your server and found in receiveOneFile:

#define BUFFER_LEN (BUFFER_SEND + 1)
  char buffer[BUFFER_LEN]= {'\0'}; ...
  string t= receiveMessage(sock);
  strcpy(buffer, t.c_str());
  int result= fwrite (buffer,1,sizeof(buffer),fp);

Select allOpen in new window

You know how many bytes you received in Message 3, but you then ignore this number of bytes received when writing to the file. This mismatch will usually cause the transferred file to have a larger size than the original. Once you fix this problem and get a good transfer for text files (that have no null bytes in them), see if the transfer for .exe files works OK. (It used to work OK - in my version - but that was before you converted char* to string and back to char*.)
=============

re: result being -1
When I wrote 4 days ago in http://rdsrc.us/OyC6hV
I assumed you did this, and since you didn't get back to me at that time, I thought you fixed it. Looks like I didn't realize you hadn't fixed it. One time when I fixed the parens issue in my version, I still got -1. Anyway, as reported in http://rdsrc.us/DkpnHB , I found it odd that by using the const size_t type instead of #define, then I got the right answer, even though it wasn't necessary in previous versions to remove the #define. That was strange - not sure why that happened. Anyway, even though it may be not necessary, in C++ we stopped using #define's at work for integral types, and also used matching data types.

=============

>> how can know it finishes reading all trunk of the file
  If you do not ignore how many bytes you receive, then you can compare the total number of bytes received in each Message 3 to the total number of bytes of the file (that you know from Message 2).

I double-checked the previous posts. In http:#35208363 sarabande addressed your question. At that point, you were expected to close the question if you understood it, and ask new questions on other issues that crop up (like -1).

   
      I understand the error in while ((result= fread (buffer, 1, BUFFER_SEND, fp) > 0)) now. It like: read,compare, and assign to result. However, I want is: read, assign, and compare. So Sara pointed out I miss the parentheses and it altered my purpose.
     
 >>>> In previous question, I posted that you were convert your file read buffer to a string and then converting it back to a char array. Very inefficient, and if null bytes are in the middle, I wonder what the effect will be when converting to a string.
    Oh sorry I forgot to mention about this comment. The reason i work around like this way is because of sendMessage(). The third argument in sendMessage() is string a. So i need to convert to match type. About the wrapper i don't know. Let me search about it. If you have a good example for this case, please post the link.
         
     

Hi Begin,

You need to switch from sending a string to sending a buffer.  Otherwise you'll be limited to sending ONLY text files.


Kent

>> The third argument in sendMessage() is string a.
Couldn't this sendMessage() call another function which is char * oriented? And when you actually have char * as in Message 3, then just use the lower level fuction.

But, as Infinity08 said in a previous question, if your OP question has been answered, we should move onto a new question. I think we have been too lax in going with the one question per question EE policy; thereby making the PAQ searches less useful.

>>>You need to switch from sending a string to sending a buffer.  Otherwise you'll be limited to sending ONLY text files.

   true.Lucky me is it transfer "client" file first. Otherwise, I can't catch the problem. Let me work on send buffer based on this idea:
>>   Couldn't this sendMessage() call another function which is char * oriented? And when you actually have char * as in Message 3, then just use the lower level fuction.

Hi Begin,

You can, maybe.  If the String uses the Z-terminator common to C, you'll have lost the length before you get to a character based object.


Kent

let me close this question first and asking new question about that new issue. . this question is  too long now. tks.

fyi - I just tested your program (with minor fixes) and at least on Cygwin, the .txt files transferred are true w.r.t. size and content.

But the .exe files although true to size are significantly different in content.

Now, given that you still are working to getting a directory structure transferred, and you have your deadlines, you need to prioritize what you consider most important to work on.

Oh i need to make it work with all type of files first. I try to finish all of them today and submit. tks a lot.

Hi Begin,

By far, the easiest way to debug this is to run both the client and server on the same box (your development system).

Then set the server to write to a relative path.  C:\servertest\....

Then it's easy to start/stop the server and application, verify file length and content, etc. all from your desktop.



Been there, done that....
Kent

>>the issue is that windows stores a pair "\r\n"for end-of-line in file but when reading it in text mode it >>returns only "\n". so the number of bytes effectively read is less than the size calculeted by ftell. same >>happens when determining the filesize with fstat or stat function. the only senseful way out (you could >>read the file twice but that rarely is an alternative) is to read (and write) the file in binary mode which >>has the advantage that by this also binary files can be transfered without difference.

>>for your client is little to change. simply open the file using "rb" openmode. then change send >>sendMessage to take a const char * and a length instead of std::string

I see . let me update my code and close this question. tks a lot.