0xFFFFFFFFUL != -1, LBER_ERROR definition wrong?

ASSIGNED
Assigned to

Status

Directory
LDAP C SDK
P2
normal
ASSIGNED
16 years ago
14 years ago

People

(Reporter: tilps, Assigned: mcs)

Tracking

other
Future
DEC
OSF/1

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: needs work)

(Reporter)

Description

16 years ago
liblber ber_printf etc all return -1 for error.
On GCC 3.2 (probably less recent versions as well) - compiling several files in
libldap produces warnings that comparison is always false, or always true
most of these are of the form LBER_ERROR == ber_printf(...)
LBER_ERROR is defined as 0xFFFFFFFFUL, and ber_printf returns an int.
LBER_ERROR is an unsigned long which is greater then MAX_INT so gcc will always
return false, for that case.

Suggest LBER_ERROR be redefined as -1
(Assignee)

Comment 1

16 years ago
LBER_ERROR is not intended to be a synonym for -1. No code should include a
check like this:

    LBER_ERROR == ber_printf(...)

Instead, the code should text against -1, e.g.,

    -1 == ber_printf(...)

Note that ber_scanf() does return LBER_ERROR upon error (and a BER tag
otherwise). I see problems like you describe in these files:

  libldap/proxyauthctrl.c
  libldap/vlistctrl.c

Did you see any others? Thanks for reporting this... I love compiler warnings!
Status: NEW → ASSIGNED
(Reporter)

Comment 2

16 years ago
ahh okay ... 

there are others of the form (berrc = ber_printf(...) ) == LBER_ERROR
where berrc is an int variable, not always called berrc
error.c is the first that comes to mind ( a few != variations as well)

a grep for all cases of LBER_ERROR in the libldap directory are almost all
incorrect (maybe not all of them now that you mention that scanf returns LBER_ERROR)
I can get a list, but not right now.
(Assignee)

Updated

16 years ago
Priority: -- → P1
(Assignee)

Updated

16 years ago
Priority: P1 → P2
Whiteboard: needs work

Comment 3

15 years ago
Spam for bug 129472
QA Contact: nobody → nobody
(Assignee)

Comment 4

14 years ago
TM = Future
Target Milestone: --- → Future
You need to log in before you can comment on or make changes to this bug.