Last Comment Bug 265715 - remove unused hsearch.c DBM code
: remove unused hsearch.c DBM code
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.9.3
: Sun SunOS
: P3 normal (vote)
: 3.12
Assigned To: Biswatosh Chakraborty
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-10-22 18:50 PDT by Julien Pierre
Modified: 2007-10-02 09:26 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch containing changes in security/dbm/src/manifest.mn. (1.39 KB, patch)
2007-09-26 07:06 PDT, Biswatosh Chakraborty
nelson: review-
Details | Diff | Splinter Review
Patch2. Shows removal of hsearch.c as well. (3.77 KB, patch)
2007-09-26 21:51 PDT, Biswatosh Chakraborty
nelson: review+
Details | Diff | Splinter Review

Description Julien Pierre 2004-10-22 18:50:32 PDT
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 Wan-Teh Chang 2004-10-23 15:08:04 PDT
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);
Comment 2 Julien Pierre 2007-09-13 15:30:03 PDT
Since no one else is using DBM except NSS now, and it doesn't call hcreate, I believe we can just delete this function.
Comment 3 Biswatosh Chakraborty 2007-09-26 07:01:28 PDT
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.


Comment 4 Biswatosh Chakraborty 2007-09-26 07:06:03 PDT
Created attachment 282401 [details] [diff] [review]
Patch containing changes in security/dbm/src/manifest.mn.

Please refer comment #3 for details on this attachment.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2007-09-26 11:28:12 PDT
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.
Comment 6 Biswatosh Chakraborty 2007-09-26 21:51:47 PDT
Created attachment 282504 [details] [diff] [review]
Patch2. Shows removal of hsearch.c as well.

Nelson,

I did the steps you suggested and now the patch shows
change of the manifest and removal of hsearch.c. Is it 
OK?
Comment 7 Nelson Bolyard (seldom reads bugmail) 2007-09-27 11:28:39 PDT
Comment on attachment 282504 [details] [diff] [review]
Patch2. Shows removal of hsearch.c as well.

r=nelson
Comment 8 Biswatosh Chakraborty 2007-10-01 22:24:43 PDT
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
Comment 9 Biswatosh Chakraborty 2007-10-01 22:33:40 PDT
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.
Comment 10 Nelson Bolyard (seldom reads bugmail) 2007-10-02 09:26:39 PDT
It was correct to not fix this on the branch, because the target fix 
milestone was 3.12, not 3.11.x

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