NXC: strncpy for identity src=dest ?

[HaWe]

I couldn't find it in the docs:
will strncpy safely work for identity src=dest?
strncpy(str,str,10);

[h-g-t]

Odd, I see strncopy in the documents on BRC but there is no reference to it at all in the usual docs!

BRC has NXC 'macros' for strcpy("dest", "src") and strncpy("dest", "src", "num") but I can't find these mentioned either.

John Hansen's guide has :-

SubStr(string, idx, len) Value
Return a sub-string from the specified input string starting at idx and including the specified number of characters.
msg = SubStr("test", 1, 2); // returns "es"

Wonder why there is such confusion?

[HaWe]

odd to me, too.
As I'm approaching NXC from the "C" side (and never from the "NX fw side" *g*) and because I'm trying to write programs which may be ported to different platforms (like C syntax makes it possible) I'm curious about this strncpy (str, str, 10) issue.

[mightor]

The SubStr() function may also be an alternative if strncpy() does not work for you.

- Xander

[HaWe]

yes, thx, I know.
But in C(C++) substr is used in a different way
string substr ( size_t pos = 0, size_t n = npos ) const;

Code: Select all

string str, str2;
str2 = str.substr (3,12);

In NXC the SubStr syntax is different from C substr and replacing substr by SubStr makes the code unreadable in "real C code".
So I'd prefer to rely on a correctly working strncpy procedure.

I couldn't find it in the docs:
will strncpy safely work for identity src=dest?
strncpy(str,str,10);

[afanofosc]

I couldn't find it in the docs:
will strncpy safely work for identity src=dest?
strncpy(str,str,10);

I don't understand why you need to ask the community this question. It is trivially easy to see for yourself whether this works or not. Based on the way that strsubset is implemented in the firmware source code it would appear that it is not safe to use the same variable as both the source and the destination since it reallocates space for the destination array before it uses memmove to copy data from the source to the desination. Since it has already reallocated the destination array which happens to also be the source array it has clobbered the original data.

Fortunately, you can easily write a portable C macro replacement that works correctly in both NXC and plain old real normal ordinary C.

And while strncpy(str, str, 10) does not work this should:

str = strncpy(tmp, str, 10);

This should work in both languages. It isn't identical in behavior, though, since the resulting string will automatically be null terminated in NXC while it will not be automatically null terminated in C.

John Hansen

[mightor]

The time spent waiting for a reply to his original question would've been longer than the time it would've taken to conduct a simple experiment.

- Xander

[HaWe]

Code: Select all

though, since the resulting string will automatically be null terminated in NXC while it will not be automatically null terminated in C.

John thx, good point that you reminded me of that!

Xander, I can't read the firmware code but I tested it three times with success astonishingly - but how could I be sure in all cases?

[afanofosc]

It may always be safe to do this since you are always allocating a smaller size so it would not need to move the array to another location. But someone would need to look more closely at the firmware source code to know for sure whether reallocating an existing array is always done in-place if the array is made smaller.

Code: Select all

NXT_STATUS cCmdDSArrayAlloc(DS_ELEMENT_ID DSElementID, UWORD Offset, UWORD NewCount)
{
  NXT_STATUS Status = NO_ERR;
  UWORD DVIndex;
  UWORD OldCount;
  UWORD i;
#if VMProfilingCode
  ULONG enterTime= dTimerReadHiRes();
#endif
  NXT_ASSERT(cCmdIsDSElementIDSane(DSElementID));

  //Only arrays are valid here
  //!!! Recommended to upgrade NXT_ASSERT to ERR_INSTR return
  NXT_ASSERT(cCmdDSType(DSElementID) == TC_ARRAY);

  DVIndex = cCmdGetDVIndex(DSElementID, Offset);
  OldCount = DV_ARRAY[DVIndex].Count;

  if(OldCount == NewCount)
    goto allocExit;
  Status = cCmdDVArrayAlloc(DVIndex, NewCount);

  if (Status < NO_ERR)
    goto allocExit;

  if(!IS_AGGREGATE_TYPE(cCmdDSType(INC_ID(DSElementID))))
    goto allocExit;

  if (OldCount > NewCount)
  {
    //Free dope vectors for sub-arrays.
    for (i = NewCount; i < OldCount; i++)
    {
      Status = cCmdFreeSubArrayDopeVectors(INC_ID(DSElementID), ARRAY_ELEM_OFFSET(DVIndex, i));
      if (IS_ERR(Status))
        goto allocExit;
    }
  }
  else if (OldCount < NewCount)
  {
    //Alloc dope vectors for sub-arrays. Set up DVIndexes
    for (i = OldCount; i < NewCount;  i++)
    {
      Status = cCmdAllocSubArrayDopeVectors(INC_ID(DSElementID), ARRAY_ELEM_OFFSET(DVIndex, i));
      if (IS_ERR(Status))
        goto allocExit;
    }
  }

  NXT_ASSERT(cCmdVerifyMemMgr());
allocExit:
#if VMProfilingCode
  memMgrTime += dTimerReadHiRes() - enterTime;
#endif
  return Status;
}

NXT_STATUS cCmdDVArrayAlloc(DV_INDEX DVIndex, UWORD NewCount)
{
  NXT_STATUS Status = NO_ERR;
  UBYTE *pData;
  UWORD ArraySize, InplaceSize;
  UWORD NextDVIndex;
  UWORD OldCount;

  OldCount = DV_ARRAY[DVIndex].Count;

  if (OldCount == NewCount)
  {
    //Nothing to alloc. Return.
    return Status;
  }
  else if (OldCount > NewCount)
  {
    //Already have the space. Shrink inplace.
    DV_ARRAY[DVIndex].Count = NewCount;
        return Status;
  }
  else // need to grow array
  {
    //Calculate new array size
    ArraySize = NewCount * DV_ARRAY[DVIndex].ElemSize;

    //Try growing inplace
    // If the Offset == NOT_AN_OFFSET then the array has never been allocated and can't grow inplace.
    if (DV_ARRAY[DVIndex].Offset != NOT_AN_OFFSET)
    {
      //Get pointer to next dope vector in dataspace
      if (DV_ARRAY[DVIndex].Link != NOT_A_DS_ID)
      {
        NextDVIndex = DV_ARRAY[DVIndex].Link;
        InplaceSize = DV_ARRAY[NextDVIndex].Offset - DV_ARRAY[DVIndex].Offset;
      }
      else
      {
        //Last element in dataspace.
        NXT_ASSERT(DVIndex == VarsCmd.MemMgr.Tail);
        InplaceSize = VarsCmd.DataspaceSize - DV_ARRAY[DVIndex].Offset;
      }

      if (ArraySize <= InplaceSize)
      {
        DV_ARRAY[DVIndex].Count = NewCount;
            return Status;
      }
    }

    //Can't grow inplace, have to allocate new space

    //Make sure we properly align for type
    //!!! This could also overflow memory (make PoolSize > POOL_MAX_SIZE) if we're within 3 bytes of the end.
    //     I don't think it matters because if it does happend, we'll trigger the ERR_MEM below and compact.
    //     During compaction, we'll reclaim these unused bytes.
    //!!! Aligning beginning of ALL arrays to 4 byte address
    ALIGN_TO_MOD(VarsCmd.PoolSize, SIZE_ULONG);
    ALIGN_TO_MOD(VarsCmd.DataspaceSize, SIZE_ULONG);

    if (VarsCmd.PoolSize + ArraySize >= POOL_MAX_SIZE)
    {
      //Not enough memory available
      return ERR_MEM;
    }

    //Get data from end of pool
    pData = VarsCmd.Pool + VarsCmd.PoolSize;
    //Grow pool and dataspace
    VarsCmd.PoolSize += ArraySize;
    VarsCmd.DataspaceSize += ArraySize;

    //Move old Array Data to new allocation
    if(OldCount)
      memmove(pData, VarsCmd.pDataspace + DV_ARRAY[DVIndex].Offset, (UWORD)(DV_ARRAY[DVIndex].ElemSize * OldCount));
    //!!! Clear mem so old mem doesn't contain stale data. Not strictly needed.
#if WIN_DEBUG || defined(ARM_DEBUG)
    memset(VarsCmd.pDataspace + DV_ARRAY[DVIndex].Offset, 0xFF, (UWORD)(DV_ARRAY[DVIndex].ElemSize * OldCount));
#endif
    //Update dope vector
    DV_ARRAY[DVIndex].Offset = pData - VarsCmd.pDataspace;
    DV_ARRAY[DVIndex].Count = NewCount;

    //Move dope vector to end of MemMgr list
    Status = cCmdMemMgrMoveToTail(DVIndex);
    if (IS_ERR(Status))
      return Status;

    NXT_ASSERT(cCmdVerifyMemMgr());
  }

  return Status;
}

Well, what do you think? I think that a closer look at the firmware source code reveals that it is always safe to use the same destination as the source with the subset opcodes (arrsubset and strsubset) since the dest will always be not longer than the source and the above functions either do nothing if the lengths are equal or they simply set the destination count to the smaller value without overwriting the existing memory.

One thing to check is whether you can grab the last couple of characters from a string (or elements in an array) and put them into the same array as the only elements when the original array is more than twice that number of elements long (i.e., so that there is no overlap between the two memory locations and the source data is beyond the end of the reallocated source array). That's not a test you can do with strncpy, of course, but it is a single line of NBC asm code.

Code: Select all

  asm { strsubset src, src, 8, 2 }

This would reallocate src, which is assumed to have at least 10 characters (plus a null) in it, to an array containing 2 characters plus a null. Does this work?

John Hansen

[HaWe]

John,
this explains why it actually worked in my previous tests - and where there could be problems in future.
thank you very much for your reply!