391609 - sftkdb.c contains too much.

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Robert Relyea]

sftkdb.c contains the following:

Handlers to manage and access the shared database (actually both databases).
String version of secmod.db.
Password handling code.

If split into three source files, it would make reading the code easier as each file would have a distinct purpose. Also there has been proposals to move secmod.db access our of softoken (which is consistant with certain FIPS goals as well). Such a move would be easier if the code were separated.

This kind of change is straightforward, but generates a pretty large diff, so I intend to make this change as a separate patch, so that other patches based in it will be easier to review.

Comment 1

[Robert Relyea]

Attachment: split sftkdb.c

patch to split sftkdb.c
This includes moving some functions to lgglue.c,
renaming some macros which have increased their reach.
 DB_ULONG_SIZE -> SDB_ULONG_SIZE
 sftk_ULong2DBULong -> sftk_ULong2SDBULong
 GET_SDB -> SFTK_GET_SDB
 item1->salt
 item2->value
 data1->saltData
 data2->valueData
 MAX_IDS->SFTK_MAX_IDS
Updating the copyright header.

Comment 2

[Robert Relyea]

Attachment: Diff between sftkmod.c and the current sftkdb.c

This patch is to help a reviewer see the changes to the new file sftkmod.c from the original sftkdb.c

Comment 3

[Robert Relyea]

Attachment: Diff between sftkpwd.c and the current sftkdb.c

This patch is to help the reviewer see the changes to sftkpwd.c from the original code in sftkdb.c

Comment 4

[Robert Relyea]

While these patches are large, they are mostly deleted code. I've attached diffs of 2 of new files against the original so real changes to those are appearant.

Comment 5

[Robert Relyea]

Comment on attachment 276198 [details] [diff] [review]
split sftkdb.c

Neil feel free to dump this review to someone else.

Comment 6

[Neil Williams]

Comment on attachment 276198 [details] [diff] [review]
split sftkdb.c

r+ with reservations about some function names. The rule for private, non-static functions seems to be something like lower_Upperletter(). 

sftk_uLong2SDBULong and some of the sftkdb_* are not static but they don't seem to follow the rule-- because they were all in sftkdb.c before?

Comment 7

[Robert Relyea]

checked in incorporation Neil's naming suggestions.

bobs-laptop(110) cvs commit
cvs commit: Examining .
Checking in lgglue.c;
/cvsroot/mozilla/security/nss/lib/softoken/lgglue.c,v  <--  lgglue.c
new revision: 1.8; previous revision: 1.7
done
Checking in manifest.mn;
/cvsroot/mozilla/security/nss/lib/softoken/manifest.mn,v  <--  manifest.mn
new revision: 1.33; previous revision: 1.32
done
Checking in sftkdb.c;
/cvsroot/mozilla/security/nss/lib/softoken/sftkdb.c,v  <--  sftkdb.c
new revision: 1.8; previous revision: 1.7
done
Checking in sftkdb.h;
/cvsroot/mozilla/security/nss/lib/softoken/sftkdb.h,v  <--  sftkdb.h
new revision: 1.3; previous revision: 1.2
done
Checking in sftkdbt.h;
/cvsroot/mozilla/security/nss/lib/softoken/sftkdbt.h,v  <--  sftkdbt.h
new revision: 1.4; previous revision: 1.3
done
RCS file: /cvsroot/mozilla/security/nss/lib/softoken/sftkdbti.h,v
done
Checking in sftkdbti.h;
/cvsroot/mozilla/security/nss/lib/softoken/sftkdbti.h,v  <--  sftkdbti.h
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/security/nss/lib/softoken/sftkmod.c,v
done
Checking in sftkmod.c;
/cvsroot/mozilla/security/nss/lib/softoken/sftkmod.c,v  <--  sftkmod.c
initial revision: 1.1
done
Checking in sftkpars.h;
/cvsroot/mozilla/security/nss/lib/softoken/sftkpars.h,v  <--  sftkpars.h
new revision: 1.3; previous revision: 1.2
done
RCS file: /cvsroot/mozilla/security/nss/lib/softoken/sftkpwd.c,v
done
Checking in sftkpwd.c;
/cvsroot/mozilla/security/nss/lib/softoken/sftkpwd.c,v  <--  sftkpwd.c
initial revision: 1.1
done