Page MenuHomeFreeBSD
Differential D49406

devel/subversion{,-lts}: Band-aid port option BDB and deprecate for removal
ClosedPublic
Actions

Authored by michaelo on Tue, Mar 18, 8:40 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 8, 4:26 PM
Unknown Object (File)
Thu, Apr 3, 7:26 AM
Unknown Object (File)
Wed, Apr 2, 3:10 PM
Unknown Object (File)
Sat, Mar 29, 1:38 PM
Unknown Object (File)
Sat, Mar 29, 7:20 AM
Unknown Object (File)
Thu, Mar 27, 6:24 PM
Unknown Object (File)
Mon, Mar 24, 9:00 PM
Unknown Object (File)
Mon, Mar 24, 9:00 PM
View All 32 Files
Subscribers
None

Details

Reviewers
jrm
otis
Commits
R11:055a1041eda4: devel/subversion{,-lts}: Band-aid port option BDB and deprecate for removal
Summary
  • Limit BDB to supported versions (4+) by Subversion only
  • Pass mandatory configure option
  • Band-aid when enabled for some combinations with devel/apr1 and bdb in DEFAULT_VERSIONS
  • Deprecate it as it is deprecated upstream already for almost 12 years and about to be removed in 1.15

PR: 285495
Tested by: michaelo
Approved by: jrm (mentor), otis (mentor)

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 62976
Build 59860: arc lint + arc unit

Event Timeline

michaelo requested review of this revision.Tue, Mar 18, 8:40 PM
michaelo created this revision.
Harbormaster completed remote builds in B62963: Diff 152398.Tue, Mar 18, 8:40 PM
jrm added inline comments.Wed, Mar 19, 3:26 AM
devel/subversion-lts/Makefile
110

That leading -a looks suspicious.

devel/subversion-lts/Makefile.common
29–31

I think you could also do:

BBDB_CONFIGURE_WITH=   berkley-db
BBDB_CONFIGURE_ENABLE= bdb6

That will turn those both off when the option is off. If you decide to go with this, please confirm it's correct.

devel/subversion/Makefile
109

Same comment about leading -a.

devel/subversion/Makefile.common
29–31

Same comment from the -lts port.

michaelo updated this revision to Diff 152410.Wed, Mar 19, 6:13 AM

Fix invalid test condition

Harbormaster completed remote builds in B62972: Diff 152410.Wed, Mar 19, 6:13 AM
michaelo added inline comments.Wed, Mar 19, 6:14 AM
devel/subversion-lts/Makefile.common
29–31

Good tip, let me evaluate this one.

michaelo updated this revision to Diff 152414.Wed, Mar 19, 7:56 AM

Add improvements from @jrm.

Harbormaster completed remote builds in B62976: Diff 152414.Wed, Mar 19, 7:56 AM
jrm accepted this revision.Thu, Mar 20, 4:32 AM
This revision is now accepted and ready to land.Thu, Mar 20, 4:32 AM
Closed by commit R11:055a1041eda4: devel/subversion{,-lts}: Band-aid port option BDB and deprecate for removal (authored by michaelo). · Explain WhyThu, Mar 20, 10:11 AM
This revision was automatically updated to reflect the committed changes.
michaelo added a commit: R11:055a1041eda4: devel/subversion{,-lts}: Band-aid port option BDB and deprecate for removal.

Revision Contents
Changeset List

PathSize
devel/
subversion-lts/
6 lines
4 lines
files/
3 lines
subversion/
6 lines
4 lines
files/
3 lines

Diff 152414

View Options

devel/subversion-lts/Makefile

Show First 20 Lines • Show All 1,358 Lines • ▼ Show 20 Lines
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
char *data = (char *)ifreqp->ifr_data; char tmpdata;
char *data;
void *info = NULL; void *info = NULL;
int retValue = EINVAL; int retValue = EINVAL;
tmpdata = fubyte(ifreqp->ifr_data);
if (tmpdata == -1)
return (EFAULT);
data = &tmpdata;
switch(*data) { switch(*data) {
jhbUnsubmitted
Not Done
Inline Actions

If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone trying to use additional bytes and instead do something like:

char query;
...

query = fubyte(ifreqp->ifr_data);
if (query == -1)
     return (EFAULT);

switch (query) {

Also, I wonder if we should prefer fuebyte() in new code instead? (Maybe kib@ has an opinion on that)

jhb: If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone…
brooksAuthorUnsubmitted
Not Done
Inline Actions

data is used later or I would have done that. We don't currently have fuebyte()

brooks: `data` is used later or I would have done that. We don't currently have fuebyte()
case XGE_QUERY_STATS: case XGE_QUERY_STATS:
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_stats_hw(lldev->devh, status = xge_hal_stats_hw(lldev->devh,
(xge_hal_stats_hw_info_t **)&info); (xge_hal_stats_hw_info_t **)&info);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(info, ifreqp->ifr_data, if(copyout(info, ifreqp->ifr_data,
sizeof(xge_hal_stats_hw_info_t)) == 0) sizeof(xge_hal_stats_hw_info_t)) == 0)
▲ Show 20 Lines • Show All 112 Lines • ▼ Show 20 Lines if(copyout(&lldev->driver_stats, ifreqp->ifr_data,
sizeof(int)) == 0) sizeof(int)) == 0)
retValue = 0; retValue = 0;
break; break;
case XGE_SET_BUFFER_MODE_1: case XGE_SET_BUFFER_MODE_1:
case XGE_SET_BUFFER_MODE_2: case XGE_SET_BUFFER_MODE_2:
case XGE_SET_BUFFER_MODE_5: case XGE_SET_BUFFER_MODE_5:
*data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N'; *data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N';
if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0)
jhbUnsubmitted
Done
Inline Actions

So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would probably fix this by not abusing data but just using a local 'char mode' and passing '&mode' to copyout().

jhb: So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would…
retValue = 0; retValue = 0;
break; break;
default: default:
xge_trace(XGE_TRACE, "Nothing is matching"); xge_trace(XGE_TRACE, "Nothing is matching");
retValue = ENOTTY; retValue = ENOTTY;
break; break;
} }
return retValue; return retValue;
} }
/** /**
* xge_ioctl_registers * xge_ioctl_registers
* IOCTL to get registers * IOCTL to get registers
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_register_t *data = (xge_register_t *)ifreqp->ifr_data; xge_register_t tmpdata;
xge_register_t *data;
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
int retValue = EINVAL, offset = 0, index = 0; int retValue = EINVAL, offset = 0, index = 0;
int error;
u64 val64 = 0; u64 val64 = 0;
error = copyin(ifreqp->ifr_data, &tmpdata, sizeof(tmpdata));
if (error != 0)
return (error);
data = &tmpdata;
/* Reading a register */ /* Reading a register */
if(strcmp(data->option, "-r") == 0) { if(strcmp(data->option, "-r") == 0) {
data->value = 0x0000; data->value = 0x0000;
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset,
&data->value); &data->value);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0)
jhbUnsubmitted
Not Done
Inline Actions

The fact that it does copyout() already makes this function even more amusing.

jhb: The fact that it does copyout() already makes this function even more amusing.
brooksAuthorUnsubmitted
Not Done
Inline Actions

The strcmp() cases will certainly not trigger in most cases because option is a char[2] followed by an int64_t so there's garbage in the pad unless the did a bzero first.

This driver is best fixed with svn rm, but I'm changing ifr_data use so needed to do a little fixing.

brooks: The strcmp() cases will certainly not trigger in most cases because option is a char[2]…
retValue = 0; retValue = 0;
} }
} }
/* Writing to a register */ /* Writing to a register */
else if(strcmp(data->option, "-w") == 0) { else if(strcmp(data->option, "-w") == 0) {
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset,
data->value); data->value);
▲ Show 20 Lines • Show All 1,982 Lines • Show Last 20 Lines
View Options

devel/subversion-lts/Makefile.common

Show First 20 Lines • Show All 1,358 Lines • ▼ Show 20 Lines
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
char *data = (char *)ifreqp->ifr_data; char tmpdata;
char *data;
void *info = NULL; void *info = NULL;
int retValue = EINVAL; int retValue = EINVAL;
tmpdata = fubyte(ifreqp->ifr_data);
if (tmpdata == -1)
return (EFAULT);
data = &tmpdata;
switch(*data) { switch(*data) {
jhbUnsubmitted
Not Done
Inline Actions

If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone trying to use additional bytes and instead do something like:

char query;
...

query = fubyte(ifreqp->ifr_data);
if (query == -1)
     return (EFAULT);

switch (query) {

Also, I wonder if we should prefer fuebyte() in new code instead? (Maybe kib@ has an opinion on that)

jhb: If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone…
brooksAuthorUnsubmitted
Not Done
Inline Actions

data is used later or I would have done that. We don't currently have fuebyte()

brooks: `data` is used later or I would have done that. We don't currently have fuebyte()
case XGE_QUERY_STATS: case XGE_QUERY_STATS:
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_stats_hw(lldev->devh, status = xge_hal_stats_hw(lldev->devh,
(xge_hal_stats_hw_info_t **)&info); (xge_hal_stats_hw_info_t **)&info);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(info, ifreqp->ifr_data, if(copyout(info, ifreqp->ifr_data,
sizeof(xge_hal_stats_hw_info_t)) == 0) sizeof(xge_hal_stats_hw_info_t)) == 0)
▲ Show 20 Lines • Show All 112 Lines • ▼ Show 20 Lines if(copyout(&lldev->driver_stats, ifreqp->ifr_data,
sizeof(int)) == 0) sizeof(int)) == 0)
retValue = 0; retValue = 0;
break; break;
case XGE_SET_BUFFER_MODE_1: case XGE_SET_BUFFER_MODE_1:
case XGE_SET_BUFFER_MODE_2: case XGE_SET_BUFFER_MODE_2:
case XGE_SET_BUFFER_MODE_5: case XGE_SET_BUFFER_MODE_5:
*data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N'; *data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N';
if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0)
jhbUnsubmitted
Done
Inline Actions

So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would probably fix this by not abusing data but just using a local 'char mode' and passing '&mode' to copyout().

jhb: So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would…
retValue = 0; retValue = 0;
break; break;
default: default:
xge_trace(XGE_TRACE, "Nothing is matching"); xge_trace(XGE_TRACE, "Nothing is matching");
retValue = ENOTTY; retValue = ENOTTY;
break; break;
} }
return retValue; return retValue;
} }
/** /**
* xge_ioctl_registers * xge_ioctl_registers
* IOCTL to get registers * IOCTL to get registers
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_register_t *data = (xge_register_t *)ifreqp->ifr_data; xge_register_t tmpdata;
xge_register_t *data;
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
int retValue = EINVAL, offset = 0, index = 0; int retValue = EINVAL, offset = 0, index = 0;
int error;
u64 val64 = 0; u64 val64 = 0;
error = copyin(ifreqp->ifr_data, &tmpdata, sizeof(tmpdata));
if (error != 0)
return (error);
data = &tmpdata;
/* Reading a register */ /* Reading a register */
if(strcmp(data->option, "-r") == 0) { if(strcmp(data->option, "-r") == 0) {
data->value = 0x0000; data->value = 0x0000;
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset,
&data->value); &data->value);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0)
jhbUnsubmitted
Not Done
Inline Actions

The fact that it does copyout() already makes this function even more amusing.

jhb: The fact that it does copyout() already makes this function even more amusing.
brooksAuthorUnsubmitted
Not Done
Inline Actions

The strcmp() cases will certainly not trigger in most cases because option is a char[2] followed by an int64_t so there's garbage in the pad unless the did a bzero first.

This driver is best fixed with svn rm, but I'm changing ifr_data use so needed to do a little fixing.

brooks: The strcmp() cases will certainly not trigger in most cases because option is a char[2]…
retValue = 0; retValue = 0;
} }
} }
/* Writing to a register */ /* Writing to a register */
else if(strcmp(data->option, "-w") == 0) { else if(strcmp(data->option, "-w") == 0) {
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset,
data->value); data->value);
▲ Show 20 Lines • Show All 1,982 Lines • Show Last 20 Lines
View Options

devel/subversion-lts/files/pkg-message.in

Show First 20 Lines • Show All 1,358 Lines • ▼ Show 20 Lines
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
char *data = (char *)ifreqp->ifr_data; char tmpdata;
char *data;
void *info = NULL; void *info = NULL;
int retValue = EINVAL; int retValue = EINVAL;
tmpdata = fubyte(ifreqp->ifr_data);
if (tmpdata == -1)
return (EFAULT);
data = &tmpdata;
switch(*data) { switch(*data) {
jhbUnsubmitted
Not Done
Inline Actions

If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone trying to use additional bytes and instead do something like:

char query;
...

query = fubyte(ifreqp->ifr_data);
if (query == -1)
     return (EFAULT);

switch (query) {

Also, I wonder if we should prefer fuebyte() in new code instead? (Maybe kib@ has an opinion on that)

jhb: If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone…
brooksAuthorUnsubmitted
Not Done
Inline Actions

data is used later or I would have done that. We don't currently have fuebyte()

brooks: `data` is used later or I would have done that. We don't currently have fuebyte()
case XGE_QUERY_STATS: case XGE_QUERY_STATS:
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_stats_hw(lldev->devh, status = xge_hal_stats_hw(lldev->devh,
(xge_hal_stats_hw_info_t **)&info); (xge_hal_stats_hw_info_t **)&info);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(info, ifreqp->ifr_data, if(copyout(info, ifreqp->ifr_data,
sizeof(xge_hal_stats_hw_info_t)) == 0) sizeof(xge_hal_stats_hw_info_t)) == 0)
▲ Show 20 Lines • Show All 112 Lines • ▼ Show 20 Lines if(copyout(&lldev->driver_stats, ifreqp->ifr_data,
sizeof(int)) == 0) sizeof(int)) == 0)
retValue = 0; retValue = 0;
break; break;
case XGE_SET_BUFFER_MODE_1: case XGE_SET_BUFFER_MODE_1:
case XGE_SET_BUFFER_MODE_2: case XGE_SET_BUFFER_MODE_2:
case XGE_SET_BUFFER_MODE_5: case XGE_SET_BUFFER_MODE_5:
*data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N'; *data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N';
if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0)
jhbUnsubmitted
Done
Inline Actions

So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would probably fix this by not abusing data but just using a local 'char mode' and passing '&mode' to copyout().

jhb: So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would…
retValue = 0; retValue = 0;
break; break;
default: default:
xge_trace(XGE_TRACE, "Nothing is matching"); xge_trace(XGE_TRACE, "Nothing is matching");
retValue = ENOTTY; retValue = ENOTTY;
break; break;
} }
return retValue; return retValue;
} }
/** /**
* xge_ioctl_registers * xge_ioctl_registers
* IOCTL to get registers * IOCTL to get registers
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_register_t *data = (xge_register_t *)ifreqp->ifr_data; xge_register_t tmpdata;
xge_register_t *data;
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
int retValue = EINVAL, offset = 0, index = 0; int retValue = EINVAL, offset = 0, index = 0;
int error;
u64 val64 = 0; u64 val64 = 0;
error = copyin(ifreqp->ifr_data, &tmpdata, sizeof(tmpdata));
if (error != 0)
return (error);
data = &tmpdata;
/* Reading a register */ /* Reading a register */
if(strcmp(data->option, "-r") == 0) { if(strcmp(data->option, "-r") == 0) {
data->value = 0x0000; data->value = 0x0000;
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset,
&data->value); &data->value);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0)
jhbUnsubmitted
Not Done
Inline Actions

The fact that it does copyout() already makes this function even more amusing.

jhb: The fact that it does copyout() already makes this function even more amusing.
brooksAuthorUnsubmitted
Not Done
Inline Actions

The strcmp() cases will certainly not trigger in most cases because option is a char[2] followed by an int64_t so there's garbage in the pad unless the did a bzero first.

This driver is best fixed with svn rm, but I'm changing ifr_data use so needed to do a little fixing.

brooks: The strcmp() cases will certainly not trigger in most cases because option is a char[2]…
retValue = 0; retValue = 0;
} }
} }
/* Writing to a register */ /* Writing to a register */
else if(strcmp(data->option, "-w") == 0) { else if(strcmp(data->option, "-w") == 0) {
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset,
data->value); data->value);
▲ Show 20 Lines • Show All 1,982 Lines • Show Last 20 Lines
View Options

devel/subversion/Makefile

Show First 20 Lines • Show All 1,358 Lines • ▼ Show 20 Lines
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_stats(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
char *data = (char *)ifreqp->ifr_data; char tmpdata;
char *data;
void *info = NULL; void *info = NULL;
int retValue = EINVAL; int retValue = EINVAL;
tmpdata = fubyte(ifreqp->ifr_data);
if (tmpdata == -1)
return (EFAULT);
data = &tmpdata;
switch(*data) { switch(*data) {
jhbUnsubmitted
Not Done
Inline Actions

If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone trying to use additional bytes and instead do something like:

char query;
...

query = fubyte(ifreqp->ifr_data);
if (query == -1)
     return (EFAULT);

switch (query) {

Also, I wonder if we should prefer fuebyte() in new code instead? (Maybe kib@ has an opinion on that)

jhb: If this is the only use, I would actually prefer to kill 'data' entirely to avoid someone…
brooksAuthorUnsubmitted
Not Done
Inline Actions

data is used later or I would have done that. We don't currently have fuebyte()

brooks: `data` is used later or I would have done that. We don't currently have fuebyte()
case XGE_QUERY_STATS: case XGE_QUERY_STATS:
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_stats_hw(lldev->devh, status = xge_hal_stats_hw(lldev->devh,
(xge_hal_stats_hw_info_t **)&info); (xge_hal_stats_hw_info_t **)&info);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(info, ifreqp->ifr_data, if(copyout(info, ifreqp->ifr_data,
sizeof(xge_hal_stats_hw_info_t)) == 0) sizeof(xge_hal_stats_hw_info_t)) == 0)
▲ Show 20 Lines • Show All 112 Lines • ▼ Show 20 Lines if(copyout(&lldev->driver_stats, ifreqp->ifr_data,
sizeof(int)) == 0) sizeof(int)) == 0)
retValue = 0; retValue = 0;
break; break;
case XGE_SET_BUFFER_MODE_1: case XGE_SET_BUFFER_MODE_1:
case XGE_SET_BUFFER_MODE_2: case XGE_SET_BUFFER_MODE_2:
case XGE_SET_BUFFER_MODE_5: case XGE_SET_BUFFER_MODE_5:
*data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N'; *data = (*data == XGE_SET_BUFFER_MODE_1) ? 'Y':'N';
if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(data)) == 0)
jhbUnsubmitted
Done
Inline Actions

So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would probably fix this by not abusing data but just using a local 'char mode' and passing '&mode' to copyout().

jhb: So this is the only place that uses 'data' and it is wrong (sizeof is wrong). I would…
retValue = 0; retValue = 0;
break; break;
default: default:
xge_trace(XGE_TRACE, "Nothing is matching"); xge_trace(XGE_TRACE, "Nothing is matching");
retValue = ENOTTY; retValue = ENOTTY;
break; break;
} }
return retValue; return retValue;
} }
/** /**
* xge_ioctl_registers * xge_ioctl_registers
* IOCTL to get registers * IOCTL to get registers
* *
* @lldev Per-adapter data * @lldev Per-adapter data
* @ifreqp Interface request * @ifreqp Interface request
*/ */
int int
xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp) xge_ioctl_registers(xge_lldev_t *lldev, struct ifreq *ifreqp)
{ {
xge_register_t *data = (xge_register_t *)ifreqp->ifr_data; xge_register_t tmpdata;
xge_register_t *data;
xge_hal_status_e status = XGE_HAL_OK; xge_hal_status_e status = XGE_HAL_OK;
int retValue = EINVAL, offset = 0, index = 0; int retValue = EINVAL, offset = 0, index = 0;
int error;
u64 val64 = 0; u64 val64 = 0;
error = copyin(ifreqp->ifr_data, &tmpdata, sizeof(tmpdata));
if (error != 0)
return (error);
data = &tmpdata;
/* Reading a register */ /* Reading a register */
if(strcmp(data->option, "-r") == 0) { if(strcmp(data->option, "-r") == 0) {
data->value = 0x0000; data->value = 0x0000;
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_read(lldev->devh, 0, data->offset,
&data->value); &data->value);
mtx_unlock(&lldev->mtx_drv); mtx_unlock(&lldev->mtx_drv);
if(status == XGE_HAL_OK) { if(status == XGE_HAL_OK) {
if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0) if(copyout(data, ifreqp->ifr_data, sizeof(xge_register_t)) == 0)
jhbUnsubmitted
Not Done
Inline Actions

The fact that it does copyout() already makes this function even more amusing.

jhb: The fact that it does copyout() already makes this function even more amusing.
brooksAuthorUnsubmitted
Not Done
Inline Actions

The strcmp() cases will certainly not trigger in most cases because option is a char[2] followed by an int64_t so there's garbage in the pad unless the did a bzero first.

This driver is best fixed with svn rm, but I'm changing ifr_data use so needed to do a little fixing.

brooks: The strcmp() cases will certainly not trigger in most cases because option is a char[2]…
retValue = 0; retValue = 0;
} }
} }
/* Writing to a register */ /* Writing to a register */
else if(strcmp(data->option, "-w") == 0) { else if(strcmp(data->option, "-w") == 0) {
mtx_lock(&lldev->mtx_drv); mtx_lock(&lldev->mtx_drv);
status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset, status = xge_hal_mgmt_reg_write(lldev->devh, 0, data->offset,
data->value); data->value);
▲ Show 20 Lines • Show All 1,982 Lines • Show Last 20 Lines
View Options

devel/subversion/Makefile.common

Loading...
View Options

devel/subversion/files/pkg-message.in

Loading...