#62489: dba_insert not working as expected #125
Conversation
|
Please add a test. |
|
@smalyshev: Tests added now The test for the inifile handler fails because this handler insert items even if the item already exists. I'm not a C developer so it would be the best merging this but leaving the bug open so another person can fix it. |
|
Hello guys, is there someone could review this ? |
| if(gdbm_store(dba->dbf, gkey, gval, | ||
| mode == 1 ? GDBM_INSERT : GDBM_REPLACE) == 0) | ||
| switch(gdbm_store(dba->dbf, gkey, gval, mode == 1 ? GDBM_INSERT : GDBM_REPLACE)) { | ||
| case -1: |
|
@marc-mabe sorry for the long delay. Please fix the coding style issues after that it looks good for merge. |
|
@lstrojny : thank you for review! Updated coding style issues now |
| case -1: | ||
| php_error_docref2(NULL TSRMLS_CC, key, val, E_WARNING, "%s", gdbm_strerror(gdbm_errno)); | ||
| return FAILURE; | ||
| default: |
There was a problem hiding this comment.
Is this really correct? Shouldn’t be the default case just throw some error?
There was a problem hiding this comment.
The default case should be impossible - it was copied from DBA_UPDATE_FUNC(flatfile)
- What should the message say on this case ?
- "Unknown return value %value% by %lib-function%" ?
- error, warning or notice?
There was a problem hiding this comment.
"Unknown return value" is fine. Warning seems appropriate to me.
|
After fixing the switch case, could you rebase and squash? |
…ected handlers are flatfile, inifile, qdbm and gdbm)
|
done! - updated the switch statements and squashed all commits together |
|
|
||
| result = dpput(dba->dbf, key, keylen, val, vallen, mode == 1 ? DP_DKEEP : DP_DOVER); | ||
| if (result) | ||
|
|
|
Merged into 5.5 and master. Cannot close it now as GitHub seems to be in trouble. |
|
Comment on behalf of lstrojny at php.net: Merged |
removed warnings if dba_insert failed because of an already existing key.
fixed handlers are: