How to improve this

[Ebenezer]

In the past I've asked for code review comments on the
code in the archive here --
http://webEbenezer.net/build_integration.html .

I didn't get a lot of response to that. Since then I've been
able to work on improving the code quite a bit. I think a lot
of what would have been mentioned previously has now been
fixed. However, I think there's still a lot of room for
improvement and so am asking again. Tia.


Brian Wood
Ebenezer Enterprises
http://webEbenezer.net

 

[Asger Joergensen]

Hi Ebenezer

In the past I've asked for code review comments on the
code in the archive here --
http://webEbenezer.net/build_integration.html .

I didn't get a lot of response to that.

Do You really find that strange ???

Ask Your self if You, without any knowledge of the project, would start
investigating in response to a post like Yours.

If You want help / suggestions, You must post some specific code and then
ask if that can be improved.

Best regards
Asger-P

 

[Ebenezer]

Hi Ebenezer



Do You really find that strange ???

Ask Your self if You, without any knowledge of the project, would start
investigating in response to a post like Yours.

If You want help / suggestions, You must post some specific code and then
ask if that can be improved.

Fortunately I have a specific question also that I posted
on another forum yesterday and haven't gotten a reply there.

class failure : public ::std::exception {
flex_string<char> whatStr;

public:
explicit failure (flex_string<char> const what_) : whatStr(what_)
{}

~failure () throw()
{}

char const* what () const throw()
{ return whatStr.c_str(); }

template <typename T>
failure& operator<< (T val)
{
::std::stringstream ss;
ss << val;
whatStr.append(ss.str().c_str());
return *this;
}
};


class eof : public ::std::exception {
flex_string<char> whatStr;

public:
explicit eof (flex_string<char> const what_) : whatStr(what_)
{}

~eof () throw()
{}

char const* what () const throw()
{ return whatStr.c_str(); }
};


I thought of changing the second class, eof, like this:

class eof : public failure {

public:
explicit eof (flex_string<char> const what_) : failure(what_)
{}

~eof () throw()
{}
};


That seems better, but two executables each become a couple
hundred bytes larger with that latter approach. The increase
in the size of the executables happens with g++ 4.5.1 and 4.6.2.
One question would be which version of eof would you use?
I'm inclined to stick with the original/longer version, because
the compilers I've checked handle it better.

 

[Pavel]

Fortunately I have a specific question also that I posted
on another forum yesterday and haven't gotten a reply there.

class failure : public ::std::exception {
flex_string<char> whatStr;

public:
explicit failure (flex_string<char> const what_) : whatStr(what_)
{}

~failure () throw()
{}

char const* what () const throw()
{ return whatStr.c_str(); }

template<typename T>
failure& operator<< (T val)
{
::std::stringstream ss;
ss<< val;
whatStr.append(ss.str().c_str());
return *this;
}
};


class eof : public ::std::exception {
flex_string<char> whatStr;

public:
explicit eof (flex_string<char> const what_) : whatStr(what_)
{}

~eof () throw()
{}

char const* what () const throw()
{ return whatStr.c_str(); }
};


I thought of changing the second class, eof, like this:

class eof : public failure {

public:
explicit eof (flex_string<char> const what_) : failure(what_)
{}

~eof () throw()
{}
};


That seems better, but two executables each become a couple
hundred bytes larger with that latter approach. The increase
in the size of the executables happens with g++ 4.5.1 and 4.6.2.
One question would be which version of eof would you use?
I'm inclined to stick with the original/longer version, because
the compilers I've checked handle it better.

Well, the answer depends on whether you mean eof to be a failure or no. If yes,
the second is appropriate; if not, either consider just re-using std:string
"what" from std::exception or, if you are 100% sure your flex_string does
something absolutely unique (and cannot even be wrapped around std::string),
create a common base class and derive eof and failure from it.

As for the executable size growth -- try 'objdump' and see if the code or data
size is *really* higher and from where exactly the increase comes; a part of
your observed decrease might be just longer mangled names that won't affect the
memory footprint of the loaded program. Some real increase may come from
implementing under-the-hood functions for virtual destructors but I would not
expect it to be in hundred bytes. Some unfortunate alignment to cache line
boundaries may be responsible; but that can play out in an opposite way in a
future build of your app so I would not worry about it.

HTH
-Pavel

 

[Ebenezer]

Well, the answer depends on whether you mean eof to be a failure or no. If yes,
the second is appropriate; if not, either consider just re-using std:string
"what" from std::exception or, if you are 100% sure your flex_string does
something absolutely unique (and cannot even be wrapped around std::string),
create a common base class and derive eof and failure from it.

I'm OK with eof being derived from failure except for this matter.

As for the executable size growth -- try 'objdump' and see if the code ordata
size is *really* higher and from where exactly the increase comes; a partof
your observed decrease might be just longer mangled names that won't affect the
memory footprint of the loaded program. Some real increase may come from
implementing under-the-hood functions for virtual destructors but I wouldnot
expect it to be in hundred bytes. Some unfortunate alignment to cache line
boundaries may be responsible; but that can play out in an opposite way in a
future build of your app so I would not worry about it.

OK, so one of the two executables that I mentioned becomes 260 bytes
larger when I derive eof from failure -- I'm using g++ 4.5.1 at
the moment. The <'s are for the original and the >'s for the
version derived from failure.

< 12 .text 0001235c 08049b40 08049b40 00001b40 2**4
---

12 .text 0001246c 08049b40 08049b40 00001b40 2**4

90c90

[ ... ]

< 15 .eh_frame_hdr 00000434 0805e6bc 0805e6bc 000166bc 2**2
---

15 .eh_frame_hdr 0000042c 0805e7dc 0805e7dc 000167dc 2**2

96c96
< 16 .eh_frame 00001740 0805eaf0 0805eaf0 00016af0 2**2
---

16 .eh_frame 00001720 0805ec08 0805ec08 00016c08 2**2

98c98
< 17 .gcc_except_table 00001203 08060230 08060230 00018230 2**2
---

17 .gcc_except_table 0000120f 08060328 08060328 00018328 2**2

I've copied what seemed relevant. It looks like the version
derived from failure leads to smaller exception related sections,
but a larger text section.

 

[Pavel]

Well, the answer depends on whether you mean eof to be a failure or no. If yes,
the second is appropriate; if not, either consider just re-using std:string
"what" from std::exception or, if you are 100% sure your flex_string does
something absolutely unique (and cannot even be wrapped around std::string),
create a common base class and derive eof and failure from it.

I'm OK with eof being derived from failure except for this matter.

As for the executable size growth -- try 'objdump' and see if the code or data
size is *really* higher and from where exactly the increase comes; a part of
your observed decrease might be just longer mangled names that won't affect the
memory footprint of the loaded program. Some real increase may come from
implementing under-the-hood functions for virtual destructors but I would not
expect it to be in hundred bytes. Some unfortunate alignment to cache line
boundaries may be responsible; but that can play out in an opposite way in a
future build of your app so I would not worry about it.

OK, so one of the two executables that I mentioned becomes 260 bytes
larger when I derive eof from failure -- I'm using g++ 4.5.1 at
the moment. The<'s are for the original and the>'s for the
version derived from failure.

< 12 .text 0001235c 08049b40 08049b40 00001b40 2**4
---

12 .text 0001246c 08049b40 08049b40 00001b40 2**4

90c90

[ ... ]

< 15 .eh_frame_hdr 00000434 0805e6bc 0805e6bc 000166bc 2**2
---

15 .eh_frame_hdr 0000042c 0805e7dc 0805e7dc 000167dc 2**2

96c96
< 16 .eh_frame 00001740 0805eaf0 0805eaf0 00016af0 2**2
---

16 .eh_frame 00001720 0805ec08 0805ec08 00016c08 2**2

98c98
< 17 .gcc_except_table 00001203 08060230 08060230 00018230 2**2
---

17 .gcc_except_table 0000120f 08060328 08060328 00018328 2**2

I've copied what seemed relevant. It looks like the version
derived from failure leads to smaller exception related sections,
but a larger text section.

It seems like more code is generated. I would use -d flag and see the
differences in the assembler code.

HTH
-Pavel

 

[Ebenezer]

class eof : public ::std::exception { [...]
I thought of changing the second class, eof, like this:

class eof : public failure {
That seems better, but two executables each become a couple
hundred bytes larger with that latter approach.

So what?

One question would be which version of eof would you use?

Neither. If EOF is an expected condition it should be handled locally and
thus does not warrant an exception. If it is an error, then an exception
like CorruptFile() should be thrown instead.

Here's where I throw the exception:

int
Read (int sock, void* data, int len)
{
int rc = read(sock
, static_cast<unsigned char*> (data)
, len
);
if (rc < 0) {
if (ECONNRESET == errno) {
throw eof("Read -- read -- errno == ECONNRESET");
}
throw failure("Read -- read len: -- errno: ") << len << " -- "
<< errno;
} else {
if (rc == 0) {
throw eof("Read -- read -- rc == 0");
}
return rc;
}
}

The name of the exception should maybe be eos for end of stream.
I don't see how it can be handled locally.