«

On Sep 18, 11:43*pm, Ian Collins <ian-n...@hotmail.com> wrote:
> On 09/19/11 01:25 PM, Ebenezer wrote:> On Sep 18, 4:34 pm, Ian Collins<ian-n...@hotmail.com> *wrote:
> >> On 09/19/11 08:30 AM, Ebenezer wrote:
>
> Why does google put in these huge gaps? *I didn't include it.
>

Not sure. Sorry.

>
>
>
> >>> On Sep 15, 7:23 pm, Ebenezer<woodbria...@gmail.com> * *wrote:
>
> >>>> ending:
> >>>> * * buf_.erase(buf_.begin(), buf_.begin() + numWritten);
> >>>> * * if (numWritten == all) return true;
>
> >>>> * * partialFlush = true;
> >>>> * * return false;
> >>>> }
>
> >>> How would you rate this function -- with a 1 being terrible
> >>> and a 10 being excellent?
>
> >> You don't appear to have headed any of the suggestions.
>
> > I'm not opposed to the size_t suggestion, but am wanting to
> > focus on the bigger picture.
>
> >> At review, I'd still give it a 1 because it isn't clear what it does,
> >> why it uses fixed width types and it still contains gratuitous gotos.
>
> > All of the code is free -- gratuitous. *Meanwhile no one has
> > provided an alternative they think is better.
>
> Everything at after your ending label could be in a bool returning
> member function.
>

OK, so if I work on an implementation like that would you
suggest making some of the local variables member variables?
The added function needs to access numWritten, all and partialFlush.

On 09/20/11 06:16 AM, Ebenezer wrote:
> On Sep 18, 11:43 pm, Ian Collins<ian-n...@hotmail.com> wrote:
>> On 09/19/11 01:25 PM, Ebenezer wrote:> On Sep 18, 4:34 pm, Ian Collins<ian-n...@hotmail.com> wrote:
>>>> On 09/19/11 08:30 AM, Ebenezer wrote:
>>>>> On Sep 15, 7:23 pm, Ebenezer<woodbria...@gmail.com> wrote:
>>
>>>>>> ending:
>>>>>> buf_.erase(buf_.begin(), buf_.begin() + numWritten);
>>>>>> if (numWritten == all) return true;
>>
>>>>>> partialFlush = true;
>>>>>> return false;
>>>>>> }
>>
>>>>> How would you rate this function -- with a 1 being terrible
>>>>> and a 10 being excellent?
>>
>>>> You don't appear to have headed any of the suggestions.
>>
>>> I'm not opposed to the size_t suggestion, but am wanting to
>>> focus on the bigger picture.
>>
>>>> At review, I'd still give it a 1 because it isn't clear what it does,
>>>> why it uses fixed width types and it still contains gratuitous gotos.
>>
>>> All of the code is free -- gratuitous. Meanwhile no one has
>>> provided an alternative they think is better.
>>
>> Everything at after your ending label could be in a bool returning
>> member function.
>>
>
> OK, so if I work on an implementation like that would you
> suggest making some of the local variables member variables?
> The added function needs to access numWritten, all and partialFlush.

Well you've already made partialFlush static, so it's a obvious
contender for a a member variable. The other two could be parameters.

--
Ian Collins

On Sep 19, 2:44*pm, Ian Collins <ian-n...@hotmail.com> wrote:
>
> Well you've already made partialFlush static, so it's a obvious
> contender for a a member variable. *The other two could be parameters.
>

bool
SendBuffer::ending (uint32_t all, uint32_t numWritten)
{
buf_.erase(buf_.begin(), buf_.begin() + numWritten);
if (numWritten == all) return true;

partialFlush_ = true;
return false;
}


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

numWritten = Write(sock_, &buf_[0], segment.size());
if (numWritten < segment.size()) return ending(all, numWritten);
partialFlush_ = false;
}

int32_t const chunks = (all - numWritten) / chunk_size;
for (int32_t ii = 0; ii < chunks; ++ii) {
uint32_t bytes = Write(sock_, &buf_[numWritten], chunk_size);
numWritten += bytes;
if (bytes < chunk_size) return ending(all, numWritten);
}

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

return ending(all, numWritten);
}


The .o file is 48 bytes bigger in this form when using
g++ 4.5.1 and -O3. I'm not sure this is a step in the
right direction.

On 09/20/11 01:03 PM, Ebenezer wrote:
> On Sep 19, 2:44 pm, Ian Collins<ian-n...@hotmail.com> wrote:
>>
>> Well you've already made partialFlush static, so it's a obvious
>> contender for a a member variable. The other two could be parameters.
>>
>
> bool
> SendBuffer::ending (uint32_t all, uint32_t numWritten)

What is your objection to size_t?

<snip>

> The .o file is 48 bytes bigger in this form when using
> g++ 4.5.1 and -O3. I'm not sure this is a step in the
> right direction.

Are you programming for a PIC? Otherwise why worry about 48 bytes?

--
Ian Collins

On Sep 19, 8:15*pm, Ian Collins <ian-n...@hotmail.com> wrote:
> On 09/20/11 01:03 PM, Ebenezer wrote:
>
> > On Sep 19, 2:44 pm, Ian Collins<ian-n...@hotmail.com> *wrote:
>
> >> Well you've already made partialFlush static, so it's a obvious
> >> contender for a a member variable. *The other two could be parameters.
>
> > bool
> > SendBuffer::ending (uint32_t all, uint32_t numWritten)
>
> What is your objection to size_t?

None really. I may change it.

>
> <snip>
>
> > The .o file is 48 bytes bigger in this form when using
> > g++ 4.5.1 and -O3. *I'm not sure this is a step in the
> > right direction.
>
> Are you programming for a PIC? *Otherwise why worry about 48 bytes?
>

I'm not worried about it, but am looking for some clues
as to whether that form is a step in the right direction.
The 48 additional bytes makes me wonder a little.