3
\$\begingroup\$

Please excuse my potentially noob-ish question. I haven't touched strings in C (or C in general) in a while, and I seem to recall there being different ways you can return a C string from a function (as discussed here).

I am currently implementing a weird driver API that I inherited. Long story short, I get passed in a "device index", and then return the device name that will be used to open() the device.

Here is my implementation:

static const char* getDeviceName(uint8_t device_index) {
  static const char* const device_names[] =
    {"", "/dev/some_device", "/dev/some_other_device", "", "", "", "", "", ""};
  if (device_index > 8) {
    return "";
  } else {
    return device_names[device_index];
  }
}

Long story short, I am provided an index (as a uint8_t for some reason) and then need to convert it to a string. As such, I create a constant array of constant strings, which is static. Note that many of these indexes are empty since they currently do not map to a device, but may in the future.

If the device index is out of bounds, I return the empty string. If the index is in bounds, I index into my constant array and return that value.

Since my C is rusty, I'm wondering if any of the above is UB, or if there are better ways of doing this. I know I can have the function return void and have the caller pass in a char* for me to populate, but I think the above is the cleanest way, since I can do things like "open(getDeviceName(index));".

In addition to my questions about UB, I also wanted to make sure that the code is const correct.

At any rate, let me know your thoughts. Thanks in advance for the help.

\$\endgroup\$
  • 1
    \$\begingroup\$ You probably know that the static in the function declaration means something different to the static of device_names, but in case you don't: make sure that the API you're implementing doesn't require your function to have external linkage, or else remove the static from the function. \$\endgroup\$ – Toby Speight Nov 7 '17 at 9:47
4
\$\begingroup\$

Your code is completely safe and free of undefined behavior.

You have used const wherever it is useful and good style.

That's great. :)

One little source of confusion, though. The list of device names contains 9 entries, but the code compares something with an 8. This 8 is confusing since the last valid index in an array is just an implementation detail. (In other programming languages like Lua or Pascal, it could be different.) Contrary, the number of devices is more interesting to any reader. Therefore, readers expect to read that number. Whatever number you write in the code, make sure it's an interesting number.

Therefore, it's better to use the idiomatic code below:

size_t size = sizeof device_names / sizeof device_names[0];
if (device_index < size) {
    return device_names[device_index];
}
return "";

This code doesn't give any clue about the actual number of devices, which makes the code more robust when a device name is added to the list.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the help! I'm comparing against 8 since I have 9 entries and that's the maximum index I can use into the device_names array to extract a string. I like the idiomatic suggestion though. \$\endgroup\$ – It'sPete Nov 7 '17 at 17:45
  • \$\begingroup\$ @TobySpeight Thanks for the suggestion, I integrated it into my answer. \$\endgroup\$ – Roland Illig Nov 7 '17 at 20:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.