Conversion of nil string to cstring is not equal to cstring(nil) #1577

Open
gradha opened this Issue Oct 18, 2014 · 3 comments

Comments

Projects
None yet
4 participants
@gradha
Contributor

gradha commented Oct 18, 2014

The following test case shows a problematic conversion between a string Nimrod type and a cstring:

proc test() =
  var a: string
  var b = cstring(a)
  var c: cstring
  var d = cstring(nil)
  echo "Is a nil? ", a.is_nil
  echo "Is b nil? ", b.is_nil
  echo "Is c nil? ", c.is_nil
  echo "Is d nil? ", d.is_nil
  {.emit: """
  char *p = 0;
  printf("C printf is nil? => %p\n", p);
  """.}

proc printf(formatstr: cstring) {.importc: "printf", varargs,
                                  header: "<stdio.h>".}

when isMainModule:
  test()
  var a: string = nil
  printf("Nimrod printf is nil? => %p\n", a)

The result of running this program is:

Is a nil? true
Is b nil? false
Is c nil? true
Is d nil? true
C printf is nil? => 0x0
Nimrod printf is nil? => 0x10

What happens is that a string with a nil value is not converted to a cstring nil value, but rather some other pointer value. I found this while interfacing with C and having stuff crash in an API where passing a nil value as a parameter is valid. The conversion between cstring and string caused the failure.

The manual CString type section implies conversions should be equivalent, so I guess this is a bug. If not, the documentation will need updating.

@Araq

This comment has been minimized.

Show comment
Hide comment
@Araq

Araq Oct 19, 2014

Member

Well that's a tough decision. On the one hand, nil should be converted to nil, I agree. On the other hand, this implies the string->cstring conversion requires a generated check (it's a special case) which I'd like to avoid. A possible compromise is to make the conversion assert that the string is not nil. What do you think?

Member

Araq commented Oct 19, 2014

Well that's a tough decision. On the one hand, nil should be converted to nil, I agree. On the other hand, this implies the string->cstring conversion requires a generated check (it's a special case) which I'd like to avoid. A possible compromise is to make the conversion assert that the string is not nil. What do you think?

@gradha

This comment has been minimized.

Show comment
Hide comment
@gradha

gradha Oct 19, 2014

Contributor

Documentation plus assert seems ok.

Contributor

gradha commented Oct 19, 2014

Documentation plus assert seems ok.

@krux02

This comment has been minimized.

Show comment
Hide comment
@krux02

krux02 Jun 7, 2017

Contributor

Honest I doubt that adding a nil check on the string to cstring conversion will ever be a bottleneck. But if you care about performance it can be implemented jump free:

#include <stdio.h>
#include <stddef.h>

typedef struct{
  long length;
  long capacity;
  char data[];
} NimString;

char* NimString2CString(NimString* arg) {
  return (arg->data) - (arg == 0) * offsetof(NimString, data);
}

int main() {
  NimString* str = 0;
  printf("%p\n", str);
  printf("%p\n", NimString2CString(str));
}

So my recommendation is to let nil strings be converted to a NULL pointer in C.

Contributor

krux02 commented Jun 7, 2017

Honest I doubt that adding a nil check on the string to cstring conversion will ever be a bottleneck. But if you care about performance it can be implemented jump free:

#include <stdio.h>
#include <stddef.h>

typedef struct{
  long length;
  long capacity;
  char data[];
} NimString;

char* NimString2CString(NimString* arg) {
  return (arg->data) - (arg == 0) * offsetof(NimString, data);
}

int main() {
  NimString* str = 0;
  printf("%p\n", str);
  printf("%p\n", NimString2CString(str));
}

So my recommendation is to let nil strings be converted to a NULL pointer in C.

@StefanSalewski StefanSalewski referenced this issue in StefanSalewski/gintro Sep 16, 2017

Closed

Frame with "nil" as parameter #12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment