Improve return value checking and error handling.

This commit is contained in:
Jon Siwek 2013-09-24 17:38:22 -05:00
parent 4b24cebad9
commit daf5d0d098
14 changed files with 101 additions and 33 deletions

View file

@ -5435,7 +5435,7 @@ TraversalCode ListExpr::Traverse(TraversalCallback* cb) const
loop_over_list(exprs, i) loop_over_list(exprs, i)
{ {
exprs[i]->Traverse(cb); tc = exprs[i]->Traverse(cb);
HANDLE_TC_EXPR_PRE(tc); HANDLE_TC_EXPR_PRE(tc);
} }

View file

@ -28,11 +28,14 @@ bool Location::DoSerialize(SerialInfo* info) const
{ {
DO_SERIALIZE(SER_LOCATION, SerialObj); DO_SERIALIZE(SER_LOCATION, SerialObj);
info->s->WriteOpenTag("Location"); info->s->WriteOpenTag("Location");
SERIALIZE(filename);
SERIALIZE(first_line); if ( ! (SERIALIZE(filename) &&
SERIALIZE(last_line); SERIALIZE(first_line) &&
SERIALIZE(first_column); SERIALIZE(last_line) &&
SERIALIZE(last_column); SERIALIZE(first_column) &&
SERIALIZE(last_column)) )
return false;
info->s->WriteCloseTag("Location"); info->s->WriteCloseTag("Location");
return true; return true;
} }

View file

@ -6,6 +6,7 @@
#include <map> #include <map>
#include <stdio.h> #include <stdio.h>
#include <errno.h>
#include <string> #include <string>
#include <vector> #include <vector>
@ -80,7 +81,12 @@ bool LoadPolicyFileText(const char* policy_filename)
policy_files.insert(PolicyFileMap::value_type(policy_filename, pf)); policy_files.insert(PolicyFileMap::value_type(policy_filename, pf));
struct stat st; struct stat st;
fstat(fileno(f), &st); if ( fstat(fileno(f), &st) != 0 )
{
char buf[256];
strerror_r(errno, buf, sizeof(buf));
reporter->InternalError("fstat failed on %s: %s", policy_filename, buf);
}
pf->lmtime = st.st_mtime; pf->lmtime = st.st_mtime;
off_t size = st.st_size; off_t size = st.st_size;

View file

@ -319,7 +319,8 @@ public:
\ \
if ( has_it ) \ if ( has_it ) \
{ \ { \
info->s->Read(&dst, 0, "has_" #dst); \ if ( ! info->s->Read(&dst, 0, "has_" #dst) ) \
return false; \
if ( ! dst ) \ if ( ! dst ) \
return false; \ return false; \
} \ } \
@ -339,7 +340,11 @@ public:
\ \
if ( has_it ) \ if ( has_it ) \
{ \ { \
info->s->Read(&dst, 0, "has_" #dst); \ if ( ! info->s->Read(&dst, 0, "has_" #dst) ) \
{ \
delete del; \
return 0; \
} \
if ( ! dst ) \ if ( ! dst ) \
{ \ { \
delete del; \ delete del; \

View file

@ -136,7 +136,12 @@ bool Serializer::Serialize(SerialInfo* info, const char* func, val_list* args)
Write(network_time, "time"); Write(network_time, "time");
Write(a, "len"); Write(a, "len");
loop_over_list(*args, i) (*args)[i]->Serialize(info); loop_over_list(*args, i)
if ( ! (*args)[i]->Serialize(info) )
{
Error("failed");
return false;
}
WriteCloseTag("call"); WriteCloseTag("call");
WriteSeparator(); WriteSeparator();

View file

@ -179,7 +179,8 @@ unsigned int BroType::MemoryAllocation() const
bool BroType::Serialize(SerialInfo* info) const bool BroType::Serialize(SerialInfo* info) const
{ {
// We always send full types (see below). // We always send full types (see below).
SERIALIZE(true); if ( ! SERIALIZE(true) )
return false;
bool ret = SerialObj::Serialize(info); bool ret = SerialObj::Serialize(info);
return ret; return ret;

View file

@ -81,7 +81,9 @@ Val* Val::Clone() const
SerialInfo sinfo(&ss); SerialInfo sinfo(&ss);
sinfo.cache = false; sinfo.cache = false;
this->Serialize(&sinfo); if ( ! this->Serialize(&sinfo) )
return 0;
char* data; char* data;
uint32 len = form->EndWrite(&data); uint32 len = form->EndWrite(&data);
form->StartRead(data, len); form->StartRead(data, len);
@ -2326,7 +2328,7 @@ bool TableVal::DoSerialize(SerialInfo* info) const
else else
reporter->InternalError("unknown continuation state"); reporter->InternalError("unknown continuation state");
HashKey* k; HashKey* k = 0;
int count = 0; int count = 0;
assert((!info->cont.ChildSuspended()) || state->v); assert((!info->cont.ChildSuspended()) || state->v);
@ -2339,12 +2341,21 @@ bool TableVal::DoSerialize(SerialInfo* info) const
if ( ! state->c ) if ( ! state->c )
{ {
// No next one. // No next one.
SERIALIZE(false); if ( ! SERIALIZE(false) )
{
delete k;
return false;
}
break; break;
} }
// There's a value coming. // There's a value coming.
SERIALIZE(true); if ( ! SERIALIZE(true) )
{
delete k;
return false;
}
if ( state->v->Value() ) if ( state->v->Value() )
state->v->Ref(); state->v->Ref();

View file

@ -95,6 +95,18 @@ void Raw::ClosePipeEnd(int i)
pipes[i] = -1; pipes[i] = -1;
} }
bool Raw::SetFDFlags(int fd, int cmd, int flags)
{
if ( fcntl(fd, cmd, flags) != -1 )
return true;
char buf[256];
strerror_r(errno, buf, sizeof(buf));
Error(Fmt("failed to set fd flags: %s", buf));
return false;
}
bool Raw::LockForkMutex() bool Raw::LockForkMutex()
{ {
int res = pthread_mutex_lock(&fork_mutex); int res = pthread_mutex_lock(&fork_mutex);
@ -200,11 +212,13 @@ bool Raw::Execute()
ClosePipeEnd(stdout_out); ClosePipeEnd(stdout_out);
if ( Info().mode == MODE_STREAM ) if ( Info().mode == MODE_STREAM )
fcntl(pipes[stdout_in], F_SETFL, O_NONBLOCK); if ( ! SetFDFlags(pipes[stdout_in], F_SETFL, O_NONBLOCK) )
return false;
ClosePipeEnd(stdin_in); ClosePipeEnd(stdin_in);
if ( stdin_towrite ) if ( stdin_towrite )
{
// Ya, just always set this to nonblocking. we do not // Ya, just always set this to nonblocking. we do not
// want to block on a program receiving data. Note // want to block on a program receiving data. Note
// that there is a small gotcha with it. More data is // that there is a small gotcha with it. More data is
@ -213,14 +227,19 @@ bool Raw::Execute()
// mode_manual where the first write cannot write // mode_manual where the first write cannot write
// everything, the rest will be stuck in a queue that // everything, the rest will be stuck in a queue that
// is never emptied. // is never emptied.
fcntl(pipes[stdin_out], F_SETFL, O_NONBLOCK); if ( ! SetFDFlags(pipes[stdin_out], F_SETFL, O_NONBLOCK) )
return false;
}
else else
ClosePipeEnd(stdin_out); ClosePipeEnd(stdin_out);
ClosePipeEnd(stderr_out); ClosePipeEnd(stderr_out);
if ( use_stderr ) if ( use_stderr )
fcntl(pipes[stderr_in], F_SETFL, O_NONBLOCK); // true for this too. {
if ( ! SetFDFlags(pipes[stderr_in], F_SETFL, O_NONBLOCK) )
return false;
}
else else
ClosePipeEnd(stderr_in); ClosePipeEnd(stderr_in);

View file

@ -31,6 +31,7 @@ protected:
private: private:
void ClosePipeEnd(int i); void ClosePipeEnd(int i);
bool SetFDFlags(int fd, int cmd, int flags);
bool LockForkMutex(); bool LockForkMutex();
bool UnlockForkMutex(); bool UnlockForkMutex();

View file

@ -242,17 +242,6 @@ public:
* Note: Exactly one of the two FinishedRotation() methods must be * Note: Exactly one of the two FinishedRotation() methods must be
* called by a writer's implementation of DoRotate() once rotation * called by a writer's implementation of DoRotate() once rotation
* has finished. * has finished.
*
* @param new_name The filename of the rotated file.
*
* @param old_name The filename of the original file.
*
* @param open: The timestamp when the original file was opened.
*
* @param close: The timestamp when the origina file was closed.
*
* @param terminating: True if the original rotation request occured
* due to the main Bro process shutting down.
*/ */
bool FinishedRotation(); bool FinishedRotation();

View file

@ -261,7 +261,16 @@ bool Ascii::DoRotate(const char* rotated_path, double open, double close, bool t
CloseFile(close); CloseFile(close);
string nname = string(rotated_path) + "." + LogExt(); string nname = string(rotated_path) + "." + LogExt();
rename(fname.c_str(), nname.c_str());
if ( rename(fname.c_str(), nname.c_str()) != 0 )
{
char buf[256];
strerror_r(errno, buf, sizeof(buf));
Error(Fmt("failed to rename %s to %s: %s", fname.c_str(),
nname.c_str(), buf));
FinishedRotation();
return false;
}
if ( ! FinishedRotation(nname.c_str(), fname.c_str(), open, close, terminating) ) if ( ! FinishedRotation(nname.c_str(), fname.c_str(), open, close, terminating) )
{ {

View file

@ -423,7 +423,16 @@ bool DataSeries::DoRotate(const char* rotated_path, double open, double close, b
string dsname = string(Info().path) + ".ds"; string dsname = string(Info().path) + ".ds";
string nname = string(rotated_path) + ".ds"; string nname = string(rotated_path) + ".ds";
rename(dsname.c_str(), nname.c_str());
if ( rename(dsname.c_str(), nname.c_str()) != 0 )
{
char buf[256];
strerror_r(errno, buf, sizeof(buf));
Error(Fmt("failed to rename %s to %s: %s", dsname.c_str(),
nname.c_str(), buf));
FinishedRotation();
return false;
}
if ( ! FinishedRotation(nname.c_str(), dsname.c_str(), open, close, terminating) ) if ( ! FinishedRotation(nname.c_str(), dsname.c_str(), open, close, terminating) )
{ {

View file

@ -782,7 +782,10 @@ int main(int argc, char** argv)
bro_init_magic(&magic_desc_cookie, MAGIC_NONE); bro_init_magic(&magic_desc_cookie, MAGIC_NONE);
bro_init_magic(&magic_mime_cookie, MAGIC_MIME); bro_init_magic(&magic_mime_cookie, MAGIC_MIME);
sqlite3_initialize(); int r = sqlite3_initialize();
if ( r != SQLITE_OK )
reporter->Error("Failed to initialize sqlite3: %s", sqlite3_errstr(r));
// FIXME: On systems that don't provide /dev/urandom, OpenSSL doesn't // FIXME: On systems that don't provide /dev/urandom, OpenSSL doesn't
// seed the PRNG. We should do this here (but at least Linux, FreeBSD // seed the PRNG. We should do this here (but at least Linux, FreeBSD

View file

@ -935,7 +935,7 @@ static const char* check_for_dir(const char* filename, bool load_pkgs)
return copy_string(filename); return copy_string(filename);
} }
FILE* open_file(const char* filename, const char** full_filename, bool load_pkgs) static FILE* open_file(const char* filename, const char** full_filename, bool load_pkgs)
{ {
filename = check_for_dir(filename, load_pkgs); filename = check_for_dir(filename, load_pkgs);
@ -944,6 +944,13 @@ FILE* open_file(const char* filename, const char** full_filename, bool load_pkgs
FILE* f = fopen(filename, "r"); FILE* f = fopen(filename, "r");
if ( ! f )
{
char buf[256];
strerror_r(errno, buf, sizeof(buf));
reporter->Error("Failed to open file %s: %s", filename, buf);
}
delete [] filename; delete [] filename;
return f; return f;