google 代码质量堪忧

zylthinking

看了几天 android framework, 发现找出一个 bug 简直太容易了， 就这样 android 居然还能分发到整个世界， 不得不说， 咱 malloc 之后立马检查返回值的努力是多么的不可取

zylthinking

还有这样的代码

    // Count objects in range
    for (int i = 0; i < (int) size; i++) {
        size_t off = objects;
        if ((off >= offset) && (off < offset + len)) {
            if (firstIndex == -1) {
                firstIndex = i;
            }
            lastIndex = i;
        }
    }

看逻辑， 肯定 objects 内的值是递增排序的， 那么

    // Count objects in range
    for (int i = 0; i < (int) size; i++) {
         size_t off = objects;
   
        if (off < offset + len) break;

        if ((off >= offset)) {
            if (firstIndex == -1) {
                firstIndex = i;
            }
            lastIndex = i;
        }
    }

会死么

zylthinking

还有这样的代码：

status_t Parcel::growData(size_t len)
{
    size_t newSize = ((mDataSize+len)*3)/2;
    return (newSize <= mDataSize)
            ? (status_t) NO_MEMORY
            : continueWrite(newSize);
}

1， 如果考虑溢出， 则 ((mDataSize+len)*3)/2 在溢出的情况下， 也有可能 newSize > mDataSize
2. 如果不考虑溢出， 那么  return (newSize <= mDataSize)
            ? (status_t) NO_MEMORY
            : continueWrite(newSize); 难道就不是直接 return continueWrite(newSize); 就可以的？

zylthinking

status_t Parcel::appendFrom(Parcel *parcel, size_t offset, size_t len)
{
    const sp<ProcessState> proc(ProcessState::self());
    status_t err;
    uint8_t *data = parcel->mData;
    size_t *objects = parcel->mObjects;
    size_t size = parcel->mObjectsSize;
    int startPos = mDataPos;
    int firstIndex = -1, lastIndex = -2;

    if (len == 0) {
        return NO_ERROR;
    }

    // range checks against the source parcel size
    if ((offset > parcel->mDataSize)
            || (len > parcel->mDataSize)
            || (offset + len > parcel->mDataSize)) {
        return BAD_VALUE;
    }

    // Count objects in range
    for (int i = 0; i < (int) size; i++) {
        size_t off = objects;
        if ((off >= offset) && (off < offset + len)) {
            if (firstIndex == -1) {
                firstIndex = i;
            }
            lastIndex = i;
        }
    }
    int numObjects = lastIndex - firstIndex + 1;

    // grow data
    err = growData(len);
    if (err != NO_ERROR) {
        return err;
    }

    // append data
    memcpy(mData + mDataPos, data + offset, len);
    mDataPos += len;
    mDataSize += len;

    if (numObjects > 0) {
        // grow objects
        if (mObjectsCapacity < mObjectsSize + numObjects) {
            int newSize = ((mObjectsSize + numObjects)*3)/2;
            size_t *objects =
                (size_t*)realloc(mObjects, newSize*sizeof(size_t));
            if (objects == (size_t*)0) {
                return NO_MEMORY;
            }
            mObjects = objects;
            mObjectsCapacity = newSize;
        }
        
        // append and acquire objects
        int idx = mObjectsSize;
        for (int i = firstIndex; i <= lastIndex; i++) {
            size_t off = objects - offset + startPos;
            mObjects[idx++] = off;
            mObjectsSize++;

            flat_binder_object* flat
                = reinterpret_cast<flat_binder_object*>(mData + off);
            acquire_object(proc, *flat, this);

            if (flat->type == BINDER_TYPE_FD) {
                // If this is a file descriptor, we need to dup it so the
                // new Parcel now owns its own fd, and can declare that we
                // officially know we have fds.
                flat->handle = dup(flat->handle);
                flat->cookie = (void*)1;
                mHasFds = mFdsKnown = true;
            }
        }
    }

    return NO_ERROR;
}

如果标红处退出， 没有回滚之前对 data 的拷贝， 结果是一种不上不下状态

zylthinking

再看这个似乎有点意思的代码：
PAD_SIZE 返回 len + 0~3, 对 len 进行 4字节补齐； 然后 google 打算在后面的 padding 中填入不同的值来表示 padding 字节数， 然而他是这样 padding 的：
*reinterpret_cast<uint32_t*>(data+padded-4) &= mask[padded-len];
也就是说， data+padded-4 介于 data － 4 与 data - 1 之间， 也就是说， 无论如何，  所谓 padding 肯定是要与正常数据争夺至少一个字节的空间的； 我就不明白， 为什么到现在， 居然还没有机器出问题，

void* Parcel::writeInplace(size_t len)
{
    const size_t padded = PAD_SIZE(len);

    // sanity check for integer overflow
    if (mDataPos+padded < mDataPos) {
        return NULL;
    }

    if ((mDataPos+padded) <= mDataCapacity) {
restart_write:
        //printf("Writing %ld bytes, padded to %ld\n", len, padded);
        uint8_t* const data = mData+mDataPos;

        // Need to pad at end?
        if (padded != len) {
#if BYTE_ORDER == BIG_ENDIAN
            static const uint32_t mask[4] = {
                0x00000000, 0xffffff00, 0xffff0000, 0xff000000
            };
#endif
#if BYTE_ORDER == LITTLE_ENDIAN
            static const uint32_t mask[4] = {
                0x00000000, 0x00ffffff, 0x0000ffff, 0x000000ff
            };
#endif
            //printf("Applying pad mask: %p to %p\n", (void*)mask[padded-len],
            //    *reinterpret_cast<void**>(data+padded-4));
            *reinterpret_cast<uint32_t*>(data+padded-4) &= mask[padded-len];
        }

        finishWrite(padded);
        return data;
    }

    status_t err = growData(padded);
    if (err == NO_ERROR) goto restart_write;
    return NULL;
}

这是调用的difang
status_t Parcel::write(const void* data, size_t len)
{
    void* const d = writeInplace(len);
    if (d) {
        memcpy(d, data, len);
        return NO_ERROR;
    }
    return mError;
}

cokeboL

哥，你都不[code]住代码了，看起来不舒服啊

kdkgod

你可以去提交BUG

lin5161678

    // Count objects in range
    for (int i = 0; i < (int) size; i++) {
         size_t off = objects;
   
//        if (off < offset + len) break;
        if (off >= offset + len) continue; //这样才和原来的代码一个意思吧?

        if ((off >= offset)) {
            if (firstIndex == -1) {
                firstIndex = i;
            }
            lastIndex = i;
        }
    }

lin5161678

再说 如果想排除非法情况 那么
    for (int i = 0; i < (int) size; i++) {
         size_t off = objects;
   
        if (off < off || setoff >= offset + len)
continue; //这样才和原来的代码一个意思吧?

        
            if (firstIndex == -1) {
                firstIndex = i;
            }
            lastIndex = i;
        
    }

linux_c_py_php

malloc检查返回值我都觉得没用... 没等返回通常都被kill了.