Closed Bug 493912 Opened 11 years ago Closed 11 years ago

sqlite3_reset should be invoked in sdb_FindObjectsInit when error occurs

Categories

(NSS :: Libraries, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
3.12.4

People

(Reporter: hiro, Assigned: rrelyea)

Details

(Whiteboard: FIPS [Awaiting Softoken's Thaw])

Attachments

(1 file)

Attached patch A patchSplinter Review
No description provided.
Attachment #378531 - Flags: review?(rrelyea)
Before I r+ this, could you give a little more context.

Why should it be called (preferably a pointer to the sqlite documentation), and what problems are caused by not calling it.

Don't assume that I don't agree with this patch, it's just the bug has almost no information for me to evaluate it (or tell if this is an isolated incident, or if there are other places that need to be changed as well).

Note that softoken is currently frozen for FIPS validation. Even if I r+ this change it's unlikely to be included in any NSS release for a very long time unless there's a pressing issue it resolves. 

Thanks,

bob
You have sharp eyes. Actually I have never faced any issues caused by this. I noticed sqlite3_reset is missing in the function when I just read code. So I am not sure what is happened without the line.
Comment on attachment 378531 [details] [diff] [review]
A patch

OK, I see it's the only place were we don't reset before finalize. I'm guessing that the sqlite docs say we should do this, which is why I reset before all the other finalizes, so even though it doesn't appear to cause any problems, its better to be friendly to sqlite.

bob
Attachment #378531 - Flags: review?(rrelyea) → review+
Taking since Hiroyuki probably does not have NSS commit access.

bob
Assignee: nobody → rrelyea
Status: NEW → ASSIGNED
Whiteboard: FIPS [Awaiting Softoken's Thaw]
Target Milestone: --- → 3.12.4
Checking in sdb.c;
/cvsroot/mozilla/security/nss/lib/softoken/sdb.c,v  <--  sdb.c
new revision: 1.14; previous revision: 1.13
done
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Bob,

The patch that you checked in is not the one that was reviewed.
The code in version 1.14 of sdb.c looks like :

    if (findstmt) {
        sqlite3_reset(stmt);
        sqlite3_finalize(findstmt);
    }

and there is a build error because stmt is undefined.
I have fixed the build by changing smt to findstmt, as the reviewed patch did.

Checking in sdb.c;
/cvsroot/mozilla/security/nss/lib/softoken/sdb.c,v  <--  sdb.c
new revision: 1.15; previous revision: 1.14
done
Thanks Julien. I thought I had built the patch to make sure I had it correct. obviously not.

bob
You need to log in before you can comment on or make changes to this bug.