Jump to content
  • Advertisement
Sign in to follow this  
wood_brian

Goto here?

This topic is 2434 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Currently I have this:


bool
SendBuffer::Flush ()
{
uint32_t all = buf_.size();
bool fullTilt = true;
::neolib::segmented_array<unsigned char, chunk_size>::segment& segment =
segmented_iterator<unsigned char, chunk_size>(buf_.begin()).segment();

uint32_t numWritten = Write(sock_, &buf_[0], segment.size());
if (numWritten < segment.size()) fullTilt = false;

while (all - numWritten > chunk_size && fullTilt) {
uint32_t bytes = Write(sock_, &buf_[numWritten], chunk_size);
numWritten += bytes;
if (bytes < chunk_size) fullTilt = false;
}

if (numWritten < all && fullTilt) {
numWritten += Write(sock_, &buf_[numWritten], all - numWritten);
}

buf_.erase(buf_.begin(), buf_.begin() + numWritten);
return numWritten == all;
}


But am tempted to do this:


bool
SendBuffer::Flush ()
{
uint32_t all = buf_.size();
::neolib::segmented_array<unsigned char, chunk_size>::segment& segment =
segmented_iterator<unsigned char, chunk_size>(buf_.begin()).segment();

uint32_t numWritten = Write(sock_, &buf_[0], segment.size());
if (numWritten < segment.size()) goto ending;

while (all - numWritten > chunk_size) {
uint32_t bytes = Write(sock_, &buf_[numWritten], chunk_size);
numWritten += bytes;
if (bytes < chunk_size) goto ending;
}

if (numWritten < all) {
numWritten += Write(sock_, &buf_[numWritten], all - numWritten);
}

ending:
buf_.erase(buf_.begin(), buf_.begin() + numWritten);
return numWritten == all;
}


Is that so bad?

Brian Wood
Ebenezer Enterprises
http://webEbenezer.net

Share this post


Link to post
Share on other sites
Advertisement
when flushing the contents of a buffer that has a fixed chunksize that is sent, I use this

unsigned int chunkstosend = totalbytestosend/ chunk_size;// this will give you the number of chunks you will be sending
for(unsigned int i=0; i<chunkstosend; i++){
Write(sock_, &buf_[numWritten], chunk_size);
}
unsigned int remainder =totalbytestosend - (chunkstosend* chunk_size);// this is the remaining bytes to send if there are any
Write(sock_, &buf_[numWritten], remainder);// Write should check for a size of 0 before sending. If it doesnt, then check for remainder == zero before calling Write in an if statement
}


The above code works because integer division rounds down in c++

Share this post


Link to post
Share on other sites
When I encounter goto or a flag variable and am in an if-it-ain't-broke-fix-it-anyway mood, I'll often change the code to eliminate it, simply because I find it easier to read, and I hate maintaining unnecessarily convoluted code. Clearly the pedant in me has no care for whether or not it's the most perfectly efficient solution:




bool SendBuffer::Flush ()
{
uint32_t all = buf_.size();
::neolib::segmented_array<unsigned char, chunk_size>::segment& segment =
segmented_iterator<unsigned char, chunk_size>(buf_.begin()).segment();

uint32_t numWritten = Write(sock_, &buf_[0], segment.size());
if (!(numWritten < segment.size())) {
while (numWritten < all) {
uint32_t bytes = Write(sock_, &buf_[numWritten], min(chunk_size, all-numWritten));
numWritten += bytes;
if (bytes != chunk_size)
break;
}
}

buf_.erase(buf_.begin(), buf_.begin() + numWritten);
return numWritten == all;
}

Share this post


Link to post
Share on other sites
I read your code five times before I found the goto.

Given that it hurts readability that much, I'd say stick with your current solution. There is no gain to adding non-obvious control flow in this situation.

Share this post


Link to post
Share on other sites

when flushing the contents of a buffer that has a fixed chunksize that is sent, I use this




Thanks for the replies. There's a possibility of incomplete flushes. In that case the first segment may have less than chunk_size elements on subsequent calls to Flush.

Share this post


Link to post
Share on other sites
I found the code with goto in it easy to read, and I prefer it to your code with a flag. I would use capital letters for the label so it sticks out more. Perhaps you can give it a name that documents why you are doing it, like BREAK_SKIPPING_LAST_WRITE.

Share this post


Link to post
Share on other sites

How about this:

bool SendBuffer::Flush()
{
    uint32_t all = buf_.size();
    ::neolib::segmented_array<unsigned char, chunk_size>::segment& segment = segmented_iterator<unsigned char, chunk_size>(buf_.begin()).segment();

    uint32_t totalWritten = Write(sock_, &buf_[0], segment.size());

    uint32_t wroteThisTime = chunk_size;
    while((totalWritten < all) && (wroteThisTime == chunk_size))
    totalWritten += wroteThisTime = Write(sock_, &buf_[totalWritten], std::min(all - totalWritten, chunk_size));

    buf_.erase(buf_.begin(), buf_.begin() + totalWritten);
    return totalWritten == all;
}

EDIT: To new readers - This code has a bug. Read on...

Edited by vreality

Share this post


Link to post
Share on other sites

How about this:

bool SendBuffer::Flush()
{
uint32_t all = buf_.size();
::neolib::segmented_array<unsigned char, chunk_size>::segment& segment = segmented_iterator<unsigned char, chunk_size>(buf_.begin()).segment();

uint32_t totalWritten = Write(sock_, &buf_[0], segment.size());

uint32_t wroteThisTime = chunk_size;
while((totalWritten < all) && (wroteThisTime == chunk_size))
totalWritten += wroteThisTime = Write(sock_, &buf_[totalWritten], std::min(all - totalWritten, chunk_size));

buf_.erase(buf_.begin(), buf_.begin() + totalWritten);
return totalWritten == all;
}



I think that has problems. If the first call to Write returns less than the segment size, the second call to Write won't be aligned properly. This could lead to incorrect data being sent. I'm not keen on the std::min use there.

Share this post


Link to post
Share on other sites

I think that has problems. If the first call to Write returns less than the segment size, the second call to Write won't be aligned properly. This could lead to incorrect data being sent. I'm not keen on the std::min use there.
No, that's not true. The " && (wroteThisTime == chunk_size)" part in the while loop will ensure that it exits the loop it the number of bytes written is not the amount intended.

Share this post


Link to post
Share on other sites

[No, that's not true. The " && (wroteThisTime == chunk_size)" part in the while loop will ensure that it exits the loop it the number of bytes written is not the amount intended.


But it goes into the while loop at least once ...
uint32_t wroteThisTime = chunk_size;
The Write in the loop may use chunk_size as the number of bytes to write. If totalWritten isn't the index of the first element in a segment, it will send data that's past the end of the segment.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!