diff --git a/TODO b/TODO index 76e4ac74..2eec7aac 100644 --- a/TODO +++ b/TODO @@ -12,9 +12,10 @@ Version 1.5.0: * Remove ecrt_domain_state(). * Segmented Sdo downloads. -* Think over pointer return values. * Implement Sdo handlers in userspace library. * Clear slave list on link down. +* Check force_config flag before error. +* Remove allow_scanning flag. Future issues: diff --git a/include/ecrt.h b/include/ecrt.h index 434b2371..9679ed46 100644 --- a/include/ecrt.h +++ b/include/ecrt.h @@ -347,11 +347,7 @@ unsigned int ecrt_version_magic(void); * The first master has index 0, the n-th master has index n - 1. The number * of masters has to be specified when loading the master module. * - * \attention In kernel context, the returned pointer has to be checked for - * errors using the IS_ERR() macro. - * - * \return If \a IS_ERR() returns zero, the result is a pointer to the - * reserved master, otherwise, the result is an error code. + * \return Pointer to the reserved master, otherwise \a NULL. */ ec_master_t *ecrt_request_master( unsigned int master_index /**< Index of the master to request. */ diff --git a/master/cdev.c b/master/cdev.c index dcd81599..410bf58f 100644 --- a/master/cdev.c +++ b/master/cdev.c @@ -1403,7 +1403,7 @@ int ec_cdev_ioctl_request( ec_master_t *m; int ret = 0; - m = ecrt_request_master(master->index); + m = ecrt_request_master_err(master->index); if (IS_ERR(m)) { ret = PTR_ERR(m); } else { @@ -1428,9 +1428,9 @@ int ec_cdev_ioctl_create_domain( if (unlikely(!priv->requested)) return -EPERM; - domain = ecrt_master_create_domain(master); - if (!domain) - return -ENOMEM; + domain = ecrt_master_create_domain_err(master); + if (IS_ERR(domain)) + return PTR_ERR(domain); return domain->index; } @@ -1455,10 +1455,10 @@ int ec_cdev_ioctl_create_slave_config( return -EFAULT; } - sc = ecrt_master_slave_config(master, data.alias, data.position, + sc = ecrt_master_slave_config_err(master, data.alias, data.position, data.vendor_id, data.product_code); - if (!sc) - return -ENODEV; // FIXME + if (IS_ERR(sc)) + return PTR_ERR(sc); data.config_index = 0; @@ -1912,9 +1912,9 @@ int ec_cdev_ioctl_sc_create_voe_handler( up(&master->master_sem); - voe = ecrt_slave_config_create_voe_handler(sc, data.size); - if (!voe) - return -ENOMEM; + voe = ecrt_slave_config_create_voe_handler_err(sc, data.size); + if (IS_ERR(voe)) + return PTR_ERR(voe); if (copy_to_user((void __user *) arg, &data, sizeof(data))) return -EFAULT; diff --git a/master/datagram.c b/master/datagram.c index 7c69a717..06a92684 100644 --- a/master/datagram.c +++ b/master/datagram.c @@ -132,7 +132,7 @@ void ec_datagram_clear(ec_datagram_t *datagram /**< EtherCAT datagram. */) * \attention If external payload memory has been provided, no range checking * is done! * - * \return 0 in case of success, else < 0 + * \return 0 in case of success, otherwise \a -ENOMEM. */ int ec_datagram_prealloc( ec_datagram_t *datagram, /**< EtherCAT datagram. */ @@ -151,7 +151,7 @@ int ec_datagram_prealloc( if (!(datagram->data = kmalloc(size, GFP_KERNEL))) { EC_ERR("Failed to allocate %u bytes of datagram memory!\n", size); - return -1; + return -ENOMEM; } datagram->mem_size = size; diff --git a/master/globals.h b/master/globals.h index c836c608..b608ddbe 100644 --- a/master/globals.h +++ b/master/globals.h @@ -237,6 +237,8 @@ size_t ec_state_string(uint8_t, char *); ssize_t ec_mac_print(const uint8_t *, char *); int ec_mac_is_zero(const uint8_t *); +ec_master_t *ecrt_request_master_err(unsigned int); + /*****************************************************************************/ /** Code/Message pair. diff --git a/master/master.c b/master/master.c index 7be1719d..6c5e5977 100644 --- a/master/master.c +++ b/master/master.c @@ -1300,7 +1300,9 @@ int ec_master_debug_level( * Realtime interface *****************************************************************************/ -ec_domain_t *ecrt_master_create_domain(ec_master_t *master /**< master */) +ec_domain_t *ecrt_master_create_domain_err( + ec_master_t *master /**< master */ + ) { ec_domain_t *domain, *last_domain; unsigned int index; @@ -1310,7 +1312,7 @@ ec_domain_t *ecrt_master_create_domain(ec_master_t *master /**< master */) if (!(domain = (ec_domain_t *) kmalloc(sizeof(ec_domain_t), GFP_KERNEL))) { EC_ERR("Error allocating domain memory!\n"); - return NULL; + return ERR_PTR(-ENOMEM); } down(&master->master_sem); @@ -1335,6 +1337,16 @@ ec_domain_t *ecrt_master_create_domain(ec_master_t *master /**< master */) /*****************************************************************************/ +ec_domain_t *ecrt_master_create_domain( + ec_master_t *master /**< master */ + ) +{ + ec_domain_t *d = ecrt_master_create_domain_err(master); + return IS_ERR(d) ? NULL : d; +} + +/*****************************************************************************/ + int ecrt_master_activate(ec_master_t *master) { uint32_t domain_offset; @@ -1462,7 +1474,7 @@ void ecrt_master_receive(ec_master_t *master) /*****************************************************************************/ -ec_slave_config_t *ecrt_master_slave_config(ec_master_t *master, +ec_slave_config_t *ecrt_master_slave_config_err(ec_master_t *master, uint16_t alias, uint16_t position, uint32_t vendor_id, uint32_t product_code) { @@ -1488,7 +1500,7 @@ ec_slave_config_t *ecrt_master_slave_config(ec_master_t *master, " 0x%08X/0x%08X before. Now configuring with" " 0x%08X/0x%08X.\n", sc->vendor_id, sc->product_code, vendor_id, product_code); - return NULL; + return ERR_PTR(-ENOENT); } } else { if (master->debug_level) @@ -1498,7 +1510,7 @@ ec_slave_config_t *ecrt_master_slave_config(ec_master_t *master, if (!(sc = (ec_slave_config_t *) kmalloc(sizeof(ec_slave_config_t), GFP_KERNEL))) { EC_ERR("Failed to allocate memory for slave configuration.\n"); - return NULL; + return ERR_PTR(-ENOMEM); } ec_slave_config_init(sc, master, @@ -1519,6 +1531,17 @@ ec_slave_config_t *ecrt_master_slave_config(ec_master_t *master, /*****************************************************************************/ +ec_slave_config_t *ecrt_master_slave_config(ec_master_t *master, + uint16_t alias, uint16_t position, uint32_t vendor_id, + uint32_t product_code) +{ + ec_slave_config_t *sc = ecrt_master_slave_config_err(master, alias, + position, vendor_id, product_code); + return IS_ERR(sc) ? NULL : sc; +} + +/*****************************************************************************/ + void ecrt_master_callbacks(ec_master_t *master, int (*request_cb)(void *), void (*release_cb)(void *), void *cb_data) { diff --git a/master/master.h b/master/master.h index aa709d90..6fa51fa6 100644 --- a/master/master.h +++ b/master/master.h @@ -218,6 +218,10 @@ const ec_domain_t *ec_master_find_domain_const(const ec_master_t *, int ec_master_debug_level(ec_master_t *, int); +ec_domain_t *ecrt_master_create_domain_err(ec_master_t *); +ec_slave_config_t *ecrt_master_slave_config_err(ec_master_t *, uint16_t, + uint16_t, uint32_t, uint32_t); + /*****************************************************************************/ #endif diff --git a/master/module.c b/master/module.c index 7d04477c..19b12b90 100644 --- a/master/module.c +++ b/master/module.c @@ -465,7 +465,11 @@ ec_device_t *ecdev_offer( * Realtime interface *****************************************************************************/ -ec_master_t *ecrt_request_master(unsigned int master_index) +/** Request a master. + * + * Same as ecrt_request_master(), but with ERR_PTR() return value. + */ +ec_master_t *ecrt_request_master_err(unsigned int master_index) { ec_master_t *master, *errptr = NULL; @@ -532,6 +536,14 @@ ec_master_t *ecrt_request_master(unsigned int master_index) /*****************************************************************************/ +ec_master_t *ecrt_request_master(unsigned int master_index) +{ + ec_master_t *master = ecrt_request_master_err(master_index); + return IS_ERR(master) ? NULL : master; +} + +/*****************************************************************************/ + void ecrt_release_master(ec_master_t *master) { EC_INFO("Releasing master %u...\n", master->index); diff --git a/master/sdo_request.c b/master/sdo_request.c index 3e1e51ea..cb75fb48 100644 --- a/master/sdo_request.c +++ b/master/sdo_request.c @@ -117,6 +117,8 @@ void ec_sdo_request_address( /** Pre-allocates the data memory. * * If the \a mem_size is already bigger than \a size, nothing is done. + * + * \return 0 on success, otherwise -ENOMEM. */ int ec_sdo_request_alloc( ec_sdo_request_t *req, /**< Sdo request. */ @@ -130,7 +132,7 @@ int ec_sdo_request_alloc( if (!(req->data = (uint8_t *) kmalloc(size, GFP_KERNEL))) { EC_ERR("Failed to allocate %u bytes of Sdo memory.\n", size); - return -1; + return -ENOMEM; } req->mem_size = size; diff --git a/master/slave_config.c b/master/slave_config.c index efbac3c7..30df4701 100644 --- a/master/slave_config.c +++ b/master/slave_config.c @@ -734,10 +734,11 @@ int ecrt_slave_config_sdo32(ec_slave_config_t *sc, uint16_t index, /*****************************************************************************/ -ec_sdo_request_t *ecrt_slave_config_create_sdo_request(ec_slave_config_t *sc, - uint16_t index, uint8_t subindex, size_t size) +ec_sdo_request_t *ecrt_slave_config_create_sdo_request_err( + ec_slave_config_t *sc, uint16_t index, uint8_t subindex, size_t size) { ec_sdo_request_t *req; + int ret; if (sc->master->debug_level) EC_DBG("ecrt_slave_config_create_sdo_request(sc = 0x%x, " @@ -747,16 +748,17 @@ ec_sdo_request_t *ecrt_slave_config_create_sdo_request(ec_slave_config_t *sc, if (!(req = (ec_sdo_request_t *) kmalloc(sizeof(ec_sdo_request_t), GFP_KERNEL))) { EC_ERR("Failed to allocate Sdo request memory!\n"); - return NULL; + return ERR_PTR(-ENOMEM); } ec_sdo_request_init(req); ec_sdo_request_address(req, index, subindex); - if (ec_sdo_request_alloc(req, size)) { + ret = ec_sdo_request_alloc(req, size); + if (ret < 0) { ec_sdo_request_clear(req); kfree(req); - return NULL; + return ERR_PTR(ret); } // prepare data for optional writing @@ -772,10 +774,21 @@ ec_sdo_request_t *ecrt_slave_config_create_sdo_request(ec_slave_config_t *sc, /*****************************************************************************/ -ec_voe_handler_t *ecrt_slave_config_create_voe_handler(ec_slave_config_t *sc, - size_t size) +ec_sdo_request_t *ecrt_slave_config_create_sdo_request( + ec_slave_config_t *sc, uint16_t index, uint8_t subindex, size_t size) +{ + ec_sdo_request_t *s = ecrt_slave_config_create_sdo_request_err(sc, index, + subindex, size); + return IS_ERR(s) ? NULL : s; +} + +/*****************************************************************************/ + +ec_voe_handler_t *ecrt_slave_config_create_voe_handler_err( + ec_slave_config_t *sc, size_t size) { ec_voe_handler_t *voe; + int ret; if (sc->master->debug_level) EC_DBG("ecrt_slave_config_create_voe_handler(sc = 0x%x, size = %u)\n", @@ -784,12 +797,13 @@ ec_voe_handler_t *ecrt_slave_config_create_voe_handler(ec_slave_config_t *sc, if (!(voe = (ec_voe_handler_t *) kmalloc(sizeof(ec_voe_handler_t), GFP_KERNEL))) { EC_ERR("Failed to allocate Sdo request memory!\n"); - return NULL; + return ERR_PTR(-ENOMEM); } - if (ec_voe_handler_init(voe, sc, size)) { + ret = ec_voe_handler_init(voe, sc, size); + if (ret < 0) { kfree(voe); - return NULL; + return ERR_PTR(ret); } down(&sc->master->master_sem); @@ -801,6 +815,16 @@ ec_voe_handler_t *ecrt_slave_config_create_voe_handler(ec_slave_config_t *sc, /*****************************************************************************/ +ec_voe_handler_t *ecrt_slave_config_create_voe_handler( + ec_slave_config_t *sc, size_t size) +{ + ec_voe_handler_t *voe = ecrt_slave_config_create_voe_handler_err(sc, + size); + return IS_ERR(voe) ? NULL : voe; +} + +/*****************************************************************************/ + void ecrt_slave_config_state(const ec_slave_config_t *sc, ec_slave_config_state_t *state) { diff --git a/master/slave_config.h b/master/slave_config.h index 2ed2c9b1..fe0081c4 100644 --- a/master/slave_config.h +++ b/master/slave_config.h @@ -92,6 +92,11 @@ const ec_sdo_request_t *ec_slave_config_get_sdo_by_pos_const( ec_voe_handler_t *ec_slave_config_find_voe_handler(ec_slave_config_t *, unsigned int); +ec_sdo_request_t *ecrt_slave_config_create_sdo_request_err( + ec_slave_config_t *, uint16_t, uint8_t, size_t); +ec_voe_handler_t *ecrt_slave_config_create_voe_handler_err( + ec_slave_config_t *, size_t); + /*****************************************************************************/ #endif diff --git a/master/voe_handler.c b/master/voe_handler.c index 32fa1c53..d4abb9b1 100644 --- a/master/voe_handler.c +++ b/master/voe_handler.c @@ -71,6 +71,8 @@ void ec_voe_handler_state_error(ec_voe_handler_t *); /*****************************************************************************/ /** VoE handler constructor. + * + * \return Return value of ec_datagram_prealloc(). */ int ec_voe_handler_init( ec_voe_handler_t *voe, /**< VoE handler. */ @@ -87,11 +89,8 @@ int ec_voe_handler_init( voe->request_state = EC_INT_REQUEST_INIT; ec_datagram_init(&voe->datagram); - if (ec_datagram_prealloc(&voe->datagram, - size + EC_MBOX_HEADER_SIZE + EC_VOE_HEADER_SIZE)) - return -1; - - return 0; + return ec_datagram_prealloc(&voe->datagram, + size + EC_MBOX_HEADER_SIZE + EC_VOE_HEADER_SIZE); } /*****************************************************************************/