Closed
Bug 265715
Opened 20 years ago
Closed 17 years ago
remove unused hsearch.c DBM code
Categories
(NSS :: Libraries, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
3.12
People
(Reporter: julien.pierre, Assigned: biswatosh2001)
Details
Attachments
(1 file, 1 obsolete file)
3.77 KB,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
The following serious warning occurs when compiling DBM on AMD64 : ../../../dbm/src/hsearch.c: In function `hcreate': ../../../dbm/src/hsearch.c:68: warning: cast from pointer to integer of different size The code is casting a DB* to int . This is very bad and won't work on platforms where sizeof(int) != sizeof(void*). However, the good news is that the hcreate function seems to have no callers in NSS, and thus this bad code doesn't currently cause a problem.
Comment 1•20 years ago
|
||
The return value of hcreate is used as a boolean: non-zero on success and 0 on failure. If dbp is 0x100000000, (int)dbp will be 0, which is the wrong return value. A fix is the following: return (dbp ? 1: 0);
Updated•19 years ago
|
QA Contact: bishakhabanerjee → jason.m.reid
Updated•18 years ago
|
Assignee: wtchang → nobody
QA Contact: jason.m.reid → libraries
Updated•18 years ago
|
Priority: -- → P3
Reporter | ||
Comment 2•17 years ago
|
||
Since no one else is using DBM except NSS now, and it doesn't call hcreate, I believe we can just delete this function.
Updated•17 years ago
|
Assignee: nobody → biswatosh2001
Assignee | ||
Comment 3•17 years ago
|
||
Julien, We can remove hsearch.c and change the security/dbm/src/manifest.mn to delist hsearch.c from the list of source files to be compiled. I did that, it is able to build nss successfully. Just like hcreate(), other two functions in the same source file (namely hsearch() and hdestroy()) are also not called anywhere else. However, dbm/src/hash.c does define another function hdestroy(HTAB*), which is different from hdestroy(void) defined in hsearch.c. Thus, hdestroy(void), hsearch() and hcreate() defined in hsearch.c are not getting called anywhere. Hence, if comment #2 proposes removal of hcreate(), we can remove the source file dbm/src/hsearch.c as well. Attaching the patch for security/dbm/src/manifest.mn.
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•17 years ago
|
||
Please refer comment #3 for details on this attachment.
Attachment #282401 -
Flags: review?(julien.pierre.boogz)
Comment 5•17 years ago
|
||
Comment on attachment 282401 [details] [diff] [review] Patch containing changes in security/dbm/src/manifest.mn. Biswatosh, If dbm really doesn't need it, then eliminating hsearch.c altogether seems like a good idea. Your patch needs to remove it from the manifest, AND also to cvs remove the file. In your workarea, remove that file, and issue the command cvs remove hsearch.c to schedule it to be removed from the workarea when you commit. Then you make a patch that shows that it has been removed. To do that, in the dbm/src directory, use the command cvs diff -Npu5 > some_patch_file_name then attach that new patch to this bug.
Attachment #282401 -
Flags: review?(julien.pierre.boogz) → review-
Assignee | ||
Comment 6•17 years ago
|
||
Nelson, I did the steps you suggested and now the patch shows change of the manifest and removal of hsearch.c. Is it OK?
Attachment #282401 -
Attachment is obsolete: true
Attachment #282504 -
Flags: review?(nelson)
Comment 7•17 years ago
|
||
Comment on attachment 282504 [details] [diff] [review] Patch2. Shows removal of hsearch.c as well. r=nelson
Attachment #282504 -
Flags: review?(nelson) → review+
Updated•17 years ago
|
Summary: bogus cast in unused DBM code → remove unused hsearch.c DBM code
Target Milestone: --- → 3.12
Assignee | ||
Comment 8•17 years ago
|
||
Checking in manifest.mn; /cvsroot/mozilla/security/dbm/src/manifest.mn,v <-- manifest.mn new revision: 1.8; previous revision: 1.7 done Removing hsearch.c; /cvsroot/mozilla/dbm/src/hsearch.c,v <-- hsearch.c new revision: delete; previous revision: 3.3 done
Assignee | ||
Comment 9•17 years ago
|
||
Wan-Teh had told me that it is not necessary to fix non-critical bugs on the NSS_3_11_BRANCH. I think this bug is also non-critical and hence am not fixing this on the Branch. So, marking this bug as closed.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 10•17 years ago
|
||
It was correct to not fix this on the branch, because the target fix milestone was 3.12, not 3.11.x
You need to log in
before you can comment on or make changes to this bug.
Description
•