eolivelli commented on issue #244: Issue#243 - asyncAddEntry fails with NPE with LedgerHandlerAdv URL: https://github.com/apache/bookkeeper/pull/244#issuecomment-316034704 @sijie I have updated the patch with a check on LedgerHandleAdv in order to prevent the client to use addEntry (and overloaded/similar functions) without passing an entryId. I am not sure that -1 or negative values are valid entryIds, because readEntries does some check and fails the call with negative values ``` // Little sanity check if (firstEntry < 0 || firstEntry > lastEntry) { LOG.error("IncorrectParameterException on ledgerId:{} firstEntry:{} lastEntry:{}", new Object[] { ledgerId, firstEntry, lastEntry }); cb.readComplete(BKException.Code.IncorrectParameterException, this, null, ctx); return; } ``` @jvrao I have no use case I was only playing with the API and I found a NullPointerException, which IMHO is always a bad bug. The fact that LedgerHandleAdv is a subclass of LedgerHandle makes it simple for a new user to use the bad API. For me is OK to prevent calls to addEntry without entryId on LedgerHandleAdv, in fact if you are assigning the entryId externally you cannot use the internal id generator. I have updated the patch and I have added testcases. I have tried to use negative entries (and -1 too) but actually it is not possible, @jvrao can you confirm ? ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
With regards, Apache Git Services
