Page MenuHomeFreeBSD
Differential D39994

libutil: move ftime to libutil
ClosedPublic
Actions

Authored by oshogbo on May 7 2023, 11:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 23, 4:01 AM
Unknown Object (File)
Wed, Mar 19, 12:08 PM
Unknown Object (File)
Sun, Mar 16, 1:35 AM
Unknown Object (File)
Mar 10 2025, 1:39 PM
Unknown Object (File)
Feb 19 2025, 6:17 AM
Unknown Object (File)
Feb 15 2025, 4:35 PM
Unknown Object (File)
Feb 7 2025, 9:09 PM
Unknown Object (File)
Jan 25 2025, 9:03 AM
View All Files
Subscribers
guest-patmaddox
imp

Details

Reviewers
kib
olce
Commits
rGc5e0b035d9eb: libutil: move ftime to libutil
rGbb421be6c117: libutil: move ftime to libutil
Summary

It seems that there are still some applications that use ftime(3)
(for example, science/siconos, and sysutils/lcdproc). The issue
is that we don't build libcompat as a shared library anymore.
The easiest solution is to move it to libutil, until we
deprecate it for good.




This solution was proposed by kib@ in

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=257789.

Author of that bug refers to the issue with science/siconos,

and recently I had similar issue with sysutils/lcdproc.
Diff Detail
Repository 
rG FreeBSD src repository 
Lint 
Lint Passed
 
Unit 
No Test Coverage
 
Build Status 
Buildable 51363
Build 48254: arc lint + arc unit
 
Event Timeline
oshogbo created this revision.May 7 2023, 11:39 AM
Herald added a subscriber: imp.  ·  View Herald TranscriptMay 7 2023, 11:39 AM
oshogbo requested review of this revision.May 7 2023, 11:39 AM
Harbormaster completed remote builds in B51362: Diff 121641.May 7 2023, 11:39 AM
oshogbo updated this revision to Diff 121642.May 7 2023, 11:45 AM
Update description.
Harbormaster completed remote builds in B51363: Diff 121642.May 7 2023, 11:45 AM
oshogbo edited the summary of this revision. (Show Details)May 7 2023, 11:47 AM
oshogbo added a reviewer: kib.
kib added a comment.May 7 2023, 4:54 PM
The easiest solution is to move it to libc, until we deprecate it for good.



If we export something from libc, it is there forever.  AFAIR this was the main reason why I left it untouched.



Might be, move it to libutil instead?
lib/libc/compat-41/ftime.c


41
This is too rude, and in principle could cause a grief from the compiler.  Perhaps a better approach would be to change sys/timeb.h to do something like this



#if !defined(_IN_LIBC) && defined(__GNUC__)
#warning
#endif



Then define _IN_LIBC before including the header.
oshogbo retitled this revision from libc: move ftime to libc to libutil: move ftime to libutil.May 12 2023, 2:05 PM
oshogbo edited the summary of this revision. (Show Details)
oshogbo edited the summary of this revision. (Show Details)
oshogbo updated this revision to Diff 121911.May 12 2023, 2:05 PM
Move to libuitl
Harbormaster completed remote builds in B51471: Diff 121911.May 12 2023, 2:05 PM
oshogbo updated this revision to Diff 121913.May 12 2023, 2:27 PM
kib@ thank you. Moved to libuitl.
Harbormaster completed remote builds in B51473: Diff 121913.May 12 2023, 2:27 PM
kib added inline comments.May 12 2023, 2:56 PM
lib/libc/compat-41/ftime.c


39
I suggest to change the protection symbol name to _IN_LIBUTIL, this way it is more consistent with existing practice of IN_RTLD/IN_LIBSA/IN_LIBDL
oshogbo updated this revision to Diff 121960.May 15 2023, 9:04 AM
Yes, thats make sense. Thank you.
Harbormaster completed remote builds in B51499: Diff 121960.May 15 2023, 9:04 AM
kib accepted this revision.May 15 2023, 9:56 AM
kib added inline comments.
lib/libcompat/Makefile


14
Did you checked that buildworld succeed?



Anyway, eventually you might want to move ftime sources to lib/libutil.
This revision is now accepted and ready to land.May 15 2023, 9:56 AM
oshogbo marked 2 inline comments as done.May 15 2023, 11:09 AM
@kib
lib/libcompat/Makefile


14
Yes, I have.



I have moved it to the lib/libutil, maybe phabricator didn't refresh your cache or something:

{F61226090}
kib added inline comments.May 15 2023, 12:56 PM
lib/libcompat/Makefile


14
I see what you did now.  The compat/4.1 confused me, and is it needed? libutil is single-directory library, why add the complication?
guest-patmaddox added a subscriber: guest-patmaddox.Dec 13 2023, 9:50 PM
Closed by commit rGbb421be6c117: libutil: move ftime to libutil (authored by oshogbo).  ·  Explain WhyMay 29 2024, 12:35 PM
This revision was automatically updated to reflect the committed changes.
oshogbo added a commit: rGbb421be6c117: libutil: move ftime to libutil.
Herald added a reviewer: olce.  ·  View Herald TranscriptMay 29 2024, 12:35 PM
oshogbo added a commit: rGc5e0b035d9eb: libutil: move ftime to libutil.Jun 28 2024, 10:17 AM
Revision Contents
Changeset List
PathSize
lib/
libc/
1 line
compat-41/
11 lines
10 lines
libc/
compat-41/
libcompat/
4.1/
4 lines
8 lines
libcompat/
4.1/
7 lines
Diff 121642
View Options
lib/libc/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
lib/libc/compat-41/Makefile.inc
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
lib/libc/compat-41/Symbol.map
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
lib/libc/compat-41/ftime.3
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
lib/libc/compat-41/ftime.c
Loading...
View Options
lib/libcompat/4.1/ftime.3
Loading...
View Options
lib/libcompat/4.1/ftime.c
Loading...
View Options
lib/libcompat/Makefile
Loading...